[PATCH] drm/exynos: fix nested calls to lock mutex in drm resume
add Mr. Dae On Thu, May 22, 2014 at 11:16 PM, Rahul Sharma wrote: > Hi Inki, > > This is another one which has not got reviewed. Please review. > > Regards, > Rahul Sharma > > On 30 April 2014 19:11, Rahul Sharma wrote: >> From: Rahul Sharma >> >> While testing S2R on exynos board, system is hanging and >> rebooting due to nested mutex_lock calls in exynos drm >> resume callback. Changing the order of the calls is fixing >> the issue. >> >> Signed-off-by: Rahul Sharma >> --- >> Based on exynos-drm-next branch in Inki Dae's tree. >> drivers/gpu/drm/exynos/exynos_drm_drv.c |2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> index bb7dfee..2bb6233 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >> @@ -184,8 +184,8 @@ static int exynos_drm_resume(struct drm_device *dev) >> connector->funcs->dpms(connector, connector->dpms); >> } >> >> - drm_helper_resume_force_mode(dev); >> drm_modeset_unlock_all(dev); >> + drm_helper_resume_force_mode(dev); >> >> return 0; >> } >> -- >> 1.7.9.5 >> > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: wait for the completion of pending page flip
On Wed, May 22, 2013 at 5:23 PM, Tomasz Figa wrote:
> Hi Inki,
>
> On Wednesday 22 of May 2013 15:37:14 Inki Dae wrote:
> > 2013/5/22 St?phane Marchesin
> >
> > > On Tue, May 21, 2013 at 9:22 PM, Inki Dae
> wrote:
> > > > 2013/5/22 St?phane Marchesin
> > > >
> > > >> On Tue, May 21, 2013 at 1:08 AM, Inki Dae
> wrote:
> > > >> > This patch fixes the issue that drm_vblank_get() is failed.
> > > >> >
> > > >> > The issus occurs when next page flip request is tried
> > > >> > if previous page flip event wasn't completed yet and then
> > > >> > dpms became off.
> > > >> >
> > > >> > So this patch make sure that page flip event is completed
> > > >> > before dpms goes to off.
> > > >>
> > > >> Hi,
> > > >>
> > > >> This patch is a squash of the two following patches from the Chrome
> > > >> OS
> > >
> > > >> tree with the KDS bits removed and the dpms off bit added:
> > > http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.g
> > > it;a=commitdiff;h=2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=aba002da
> > > 4c6e5efec4d43e1ce33930a79269349a
> > >
> > >
> > > http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.g
> > > it;a=commitdiff;h=b4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=4f28b9a7
> > > 5c928f229443d7c6c3163159ceb6903a>
> > > >> Please keep proper attribution.
> > > >
> > > > Those patches are just for Chrome OS. Please post them if you want
> > > > for
> > >
> > > those
> > >
> > > > to be considered so that they can be reviewed.
>
Hi Stephane,
> > >
> > > We intend to, once they are rebased onto latest kernel. But what I'm
> > > pointing out is that you're removing proper attribution from Chrome OS
> > > patches, and this is the second time it has happened.
>
First, we don't know what's going on Chrome OS. probably you think we refer
your codes. but we don't know chrome codes and doesn't refer it. it's our
finding from our use case.
Second, samsung has lots of division. some engineers are working with
Chrome OS. but ours is not their division.
now we're working on anoter platform and use exynos drm.
As you know "chinese wall". we're doing it properly.
Thank you,
Kyungmin Park
> >
> > What is mean? does mainline kernel have the attribution? if not so, we
> > don't need to consider it. And please know that I can not be aware of
> > you have such patch set without any posting.
> >
>
> at?tri?bu?tion
> n.
> 1. The act of attributing, especially the act of establishing a particular
> person as the creator of a work of art
> [1]
>
> So yes, mainline kernel has attribution. Actually posting something as
> work of someone that is not the author of the posted work is considered
> bad everywhere, isn't it?
>
> However looking at those two patches linked by St?phane, I'm not really
> sure this patch is really just a squash of them. St?phane, are you 100%
> sure that your claims are true?
>
> Best regards,
> Tomasz
>
> [1] http://www.thefreedictionary.com/attribution
>
> > > > That is why we attend open
> > > > source. One more comment, please do not abuse
> > > > exynos_drm_crtc_page_flip()>
> > > What do you mean by abuse?
> > >
> > > >> Signed-off-by: Inki Dae
> > > >> Signed-off-by: Kyungmin Park
> > > >> ---
> > > >>
> > > >> drivers/gpu/drm/exynos/exynos_drm_crtc.c | 16
> > > >> 1 files changed, 16 insertions(+), 0 deletions(-)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > > >> b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > > >> index e8894bc..69a77e9 100644
> > > >> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > > >> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> > > >> @@ -48,6 +48,8 @@ struct exynos_drm_crtc {
> > > >>
> > > >> unsigned intpipe;
> > > >> unsigned intdpms;
> > > >> enum exynos_crtc_mode mode;
> > > >>
> > > >> + wait_queue_head_t pending_flip_queue;
> > > >> + atomic_tpending_flip;
> > > >>
Re: [PATCH] drm/exynos: wait for the completion of pending page flip
On Wed, May 22, 2013 at 5:23 PM, Tomasz Figa tomasz.f...@gmail.com wrote:
Hi Inki,
On Wednesday 22 of May 2013 15:37:14 Inki Dae wrote:
2013/5/22 Stéphane Marchesin stephane.marche...@gmail.com
On Tue, May 21, 2013 at 9:22 PM, Inki Dae inki@samsung.com
wrote:
2013/5/22 Stéphane Marchesin stephane.marche...@gmail.com
On Tue, May 21, 2013 at 1:08 AM, Inki Dae inki@samsung.com
wrote:
This patch fixes the issue that drm_vblank_get() is failed.
The issus occurs when next page flip request is tried
if previous page flip event wasn't completed yet and then
dpms became off.
So this patch make sure that page flip event is completed
before dpms goes to off.
Hi,
This patch is a squash of the two following patches from the Chrome
OS
tree with the KDS bits removed and the dpms off bit added:
http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.g
it;a=commitdiff;h=2e77cd4e423967862ca01b1af82aa8b5b7429fc4;hp=aba002da
4c6e5efec4d43e1ce33930a79269349a
http://git.chromium.org/gitweb/?p=chromiumos/third_party/kernel-next.g
it;a=commitdiff;h=b4ec8bfa750ef43a43c2da746c8afdbb51002882;hp=4f28b9a7
5c928f229443d7c6c3163159ceb6903a
Please keep proper attribution.
Those patches are just for Chrome OS. Please post them if you want
for
those
to be considered so that they can be reviewed.
Hi Stephane,
We intend to, once they are rebased onto latest kernel. But what I'm
pointing out is that you're removing proper attribution from Chrome OS
patches, and this is the second time it has happened.
First, we don't know what's going on Chrome OS. probably you think we refer
your codes. but we don't know chrome codes and doesn't refer it. it's our
finding from our use case.
Second, samsung has lots of division. some engineers are working with
Chrome OS. but ours is not their division.
now we're working on anoter platform and use exynos drm.
As you know chinese wall. we're doing it properly.
Thank you,
Kyungmin Park
What is mean? does mainline kernel have the attribution? if not so, we
don't need to consider it. And please know that I can not be aware of
you have such patch set without any posting.
at·tri·bu·tion
n.
1. The act of attributing, especially the act of establishing a particular
person as the creator of a work of art
[1]
So yes, mainline kernel has attribution. Actually posting something as
work of someone that is not the author of the posted work is considered
bad everywhere, isn't it?
However looking at those two patches linked by Stéphane, I'm not really
sure this patch is really just a squash of them. Stéphane, are you 100%
sure that your claims are true?
Best regards,
Tomasz
[1] http://www.thefreedictionary.com/attribution
That is why we attend open
source. One more comment, please do not abuse
exynos_drm_crtc_page_flip()
What do you mean by abuse?
Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_crtc.c | 16
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index e8894bc..69a77e9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -48,6 +48,8 @@ struct exynos_drm_crtc {
unsigned intpipe;
unsigned intdpms;
enum exynos_crtc_mode mode;
+ wait_queue_head_t pending_flip_queue;
+ atomic_tpending_flip;
};
static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
@@ -61,6 +63,13 @@ static void exynos_drm_crtc_dpms(struct drm_crtc
*crtc,
int mode)
return;
}
+ if (mode DRM_MODE_DPMS_ON) {
+ /* wait for the completion of page flip. */
+ wait_event(exynos_crtc-pending_flip_queue,
+
atomic_read(exynos_crtc-pending_flip)
==
0);
+ drm_vblank_off(crtc-dev, exynos_crtc-pipe);
You should be using vblank_put/get.
No, drm_vblank_put should be called by
exynos_drm_crtc_finish_pageflip(). And know that this patch makes
sure that pended page flip event is
completed
before dpms goes to off.
You need to do vblank_put/get while you're waiting. Otherwise you
don't know for sure that flips will happen. This is especially bad as
you don't use a timeout.
Understood. Right, drm_vblank_get call before wait_event and
drm_vblank_put call after wait_event.
Thanks,
Inki Dae
Stéphane
Thanks,
Inki Dae
Stéphane
[PATCH] MAINTAINERS: Add linux-samsung-soc list to all related entries
Hi, I don't think it's not required, each tree has each own mailing list. don't need to post all patches to samsung-soc list. Thank you, Kyungmin Park On Mon, Apr 22, 2013 at 4:20 AM, Tomasz Figa wrote: > Several entries in MAINTAINERS file related to Samsung SoCs do not point > to linux-samsung-soc mailing list, which is supposed to collect all > Samsung SoC-related threads, to ease following of Samsung SoC-related > work. This leads to a problem with people skipping this mailing list in > their posts, even though they are related to Samsung SoCs. > > This patch adds pointers to linux-samsung-soc mailing list to affected > entries of MAINTAINERS file. > > Signed-off-by: Tomasz Figa > --- > MAINTAINERS | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 6c0d68b..07cb8da 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -1139,6 +1139,7 @@ F:arch/arm/mach-exynos*/ > ARM/SAMSUNG MOBILE MACHINE SUPPORT > M: Kyungmin Park > L: linux-arm-kernel at lists.infradead.org (moderated for > non-subscribers) > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: arch/arm/mach-s5pv210/mach-aquila.c > F: arch/arm/mach-s5pv210/mach-goni.c > @@ -1150,6 +1151,7 @@ M:Kyungmin Park > M: Kamil Debski > L: linux-arm-kernel at lists.infradead.org > L: linux-media at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: drivers/media/platform/s5p-g2d/ > > @@ -1158,6 +1160,7 @@ M:Kyungmin Park > M: Sylwester Nawrocki > L: linux-arm-kernel at lists.infradead.org > L: linux-media at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: arch/arm/plat-samsung/include/plat/*fimc* > F: drivers/media/platform/s5p-fimc/ > @@ -1168,6 +1171,7 @@ M:Kamil Debski > M: Jeongtae Park > L: linux-arm-kernel at lists.infradead.org > L: linux-media at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: arch/arm/plat-samsung/s5p-dev-mfc.c > F: drivers/media/platform/s5p-mfc/ > @@ -1177,6 +1181,7 @@ M:Kyungmin Park > M: Tomasz Stanislawski > L: linux-arm-kernel at lists.infradead.org > L: linux-media at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: drivers/media/platform/s5p-tv/ > > @@ -2679,6 +2684,7 @@ M:Joonyoung Shim > M: Seung-Woo Kim > M: Kyungmin Park > L: dri-devel at lists.freedesktop.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > T: git git:// > git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git > S: Supported > F: drivers/gpu/drm/exynos > @@ -3142,6 +3148,7 @@ F:Documentation/extcon/ > EXYNOS DP DRIVER > M: Jingoo Han > L: linux-fbdev at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: drivers/video/exynos/exynos_dp* > F: include/video/exynos_dp* > @@ -3151,6 +3158,7 @@ M:Inki Dae > M: Donghwa Lee > M: Kyungmin Park > L: linux-fbdev at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: drivers/video/exynos/exynos_mipi* > F: include/video/exynos_mipi* > @@ -6892,12 +6900,14 @@ F: drivers/platform/x86/samsung-laptop.c > SAMSUNG AUDIO (ASoC) DRIVERS > M: Sangbeom Kim > L: alsa-devel at alsa-project.org (moderated for non-subscribers) > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Supported > F: sound/soc/samsung > > SAMSUNG FRAMEBUFFER DRIVER > M: Jingoo Han > L: linux-fbdev at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: drivers/video/s3c-fb.c > > @@ -7087,6 +7097,7 @@ F:drivers/mmc/host/sdhci-pltfm.[ch] > SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) SAMSUNG DRIVER > M: Ben Dooks > L: linux-mmc at vger.kernel.org > +L: linux-samsung-soc at vger.kernel.org (moderated for non-subscribers) > S: Maintained > F: drivers/mmc/host/sdhci-s3c.c > > -- > 1.8.2.1 > > > ___ > linux-arm-kernel mailing list > linux-arm-kernel at lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20130422/307083cb/attachment.html>
Re: [PATCH] MAINTAINERS: Add linux-samsung-soc list to all related entries
Hi, I don't think it's not required, each tree has each own mailing list. don't need to post all patches to samsung-soc list. Thank you, Kyungmin Park On Mon, Apr 22, 2013 at 4:20 AM, Tomasz Figa tomasz.f...@gmail.com wrote: Several entries in MAINTAINERS file related to Samsung SoCs do not point to linux-samsung-soc mailing list, which is supposed to collect all Samsung SoC-related threads, to ease following of Samsung SoC-related work. This leads to a problem with people skipping this mailing list in their posts, even though they are related to Samsung SoCs. This patch adds pointers to linux-samsung-soc mailing list to affected entries of MAINTAINERS file. Signed-off-by: Tomasz Figa tomasz.f...@gmail.com --- MAINTAINERS | 11 +++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 6c0d68b..07cb8da 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1139,6 +1139,7 @@ F:arch/arm/mach-exynos*/ ARM/SAMSUNG MOBILE MACHINE SUPPORT M: Kyungmin Park kyungmin.p...@samsung.com L: linux-arm-ker...@lists.infradead.org (moderated for non-subscribers) +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: arch/arm/mach-s5pv210/mach-aquila.c F: arch/arm/mach-s5pv210/mach-goni.c @@ -1150,6 +1151,7 @@ M:Kyungmin Park kyungmin.p...@samsung.com M: Kamil Debski k.deb...@samsung.com L: linux-arm-ker...@lists.infradead.org L: linux-me...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: drivers/media/platform/s5p-g2d/ @@ -1158,6 +1160,7 @@ M:Kyungmin Park kyungmin.p...@samsung.com M: Sylwester Nawrocki s.nawro...@samsung.com L: linux-arm-ker...@lists.infradead.org L: linux-me...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: arch/arm/plat-samsung/include/plat/*fimc* F: drivers/media/platform/s5p-fimc/ @@ -1168,6 +1171,7 @@ M:Kamil Debski k.deb...@samsung.com M: Jeongtae Park jtp.p...@samsung.com L: linux-arm-ker...@lists.infradead.org L: linux-me...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: arch/arm/plat-samsung/s5p-dev-mfc.c F: drivers/media/platform/s5p-mfc/ @@ -1177,6 +1181,7 @@ M:Kyungmin Park kyungmin.p...@samsung.com M: Tomasz Stanislawski t.stanisl...@samsung.com L: linux-arm-ker...@lists.infradead.org L: linux-me...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: drivers/media/platform/s5p-tv/ @@ -2679,6 +2684,7 @@ M:Joonyoung Shim jy0922.s...@samsung.com M: Seung-Woo Kim sw0312@samsung.com M: Kyungmin Park kyungmin.p...@samsung.com L: dri-devel@lists.freedesktop.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) T: git git:// git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git S: Supported F: drivers/gpu/drm/exynos @@ -3142,6 +3148,7 @@ F:Documentation/extcon/ EXYNOS DP DRIVER M: Jingoo Han jg1@samsung.com L: linux-fb...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: drivers/video/exynos/exynos_dp* F: include/video/exynos_dp* @@ -3151,6 +3158,7 @@ M:Inki Dae inki@samsung.com M: Donghwa Lee dh09@samsung.com M: Kyungmin Park kyungmin.p...@samsung.com L: linux-fb...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: drivers/video/exynos/exynos_mipi* F: include/video/exynos_mipi* @@ -6892,12 +6900,14 @@ F: drivers/platform/x86/samsung-laptop.c SAMSUNG AUDIO (ASoC) DRIVERS M: Sangbeom Kim sbki...@samsung.com L: alsa-de...@alsa-project.org (moderated for non-subscribers) +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Supported F: sound/soc/samsung SAMSUNG FRAMEBUFFER DRIVER M: Jingoo Han jg1@samsung.com L: linux-fb...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: drivers/video/s3c-fb.c @@ -7087,6 +7097,7 @@ F:drivers/mmc/host/sdhci-pltfm.[ch] SECURE DIGITAL HOST CONTROLLER INTERFACE (SDHCI) SAMSUNG DRIVER M: Ben Dooks ben-li...@fluff.org L: linux-...@vger.kernel.org +L: linux-samsung-...@vger.kernel.org (moderated for non-subscribers) S: Maintained F: drivers/mmc/host/sdhci-s3c.c -- 1.8.2.1 ___ linux-arm-kernel mailing list linux-arm-ker...@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
On Wed, Feb 6, 2013 at 9:42 AM, Stephen Warren wrote:
> On 02/05/2013 05:37 PM, Sean Paul wrote:
>> On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren
>> wrote:
>>> n 02/05/2013 04:42 PM, Sean Paul wrote:
>>>> Use the compatible string in the device tree to determine which
>>>> registers/functions to use in the HDMI driver. Also changes the
>>>> references from v13 to 4210 and v14 to 4212 to reflect the IP
>>>> block version instead of the HDMI version.
>>>
>>>> diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
>>>> b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
>>>
>>> Binding looks sane to me.
>>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>> b/drivers/gpu/drm/exynos/exynos_hdmi.c
>>>
>>>> #ifdef CONFIG_OF
>>>> static struct of_device_id hdmi_match_types[] = {
>>>> {
>>>> - .compatible = "samsung,exynos5-hdmi",
>>>> - .data = (void *)HDMI_TYPE14,
>>>> + .compatible = "samsung,exynos4-hdmi",
>>>> }, {
>>>> /* end node */
>>>> }
>>>
>>> Why not fill in all the "base" compatible values there (I think you need
>>> this anyway so that DTs don't all have to be compatible with
>>> samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
>>> values, then ...
>>>
>>
>> At the moment, all DTs have to be compatible with exynos4-hdmi since
>> it provides the base for the current driver. The driver uses 4210 and
>> 4212 to differentiate between different register addresses and
>> features, but most things are just exynos4-hdmi compatible.
I would like to distinguish between 4210 and 4x12. since it has
different features. e.g., HDMI v1.3 and v1.4.
and I also want to use 4412 instead of 4212. there's no board to use
4212 at mainline until this time.
Thank you,
Kyungmin Park
>
> The DT nodes should include only the compatible values that the HW is
> actually compatible with. If the HW isn't compatible with exynos4-hdmi,
> that value shouldn't be in the compatible property, but instead whatever
> the "base" value that the HW really is compatible with. The driver can
> support multiple "base" compatible values from this table.
>
>>>> @@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
>>>
>>>> +
>>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4210-hdmi"))
>>>> + hdata->version |= HDMI_VER_EXYNOS4210;
>>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos4212-hdmi"))
>>>> + hdata->version |= HDMI_VER_EXYNOS4212;
>>>> + if (of_device_is_compatible(dev->of_node, "samsung,exynos5250-hdmi"))
>>>> + hdata->version |= HDMI_VER_EXYNOS5250;
>>>
>>> Instead of that, do roughly:
>>>
>>> match = of_match_device(hdmi_match_types, >dev);
>>> if (match)
>>> hdata->version |= (int)match->data;
>>>
>>> That way, it's all table-based. Any future additions to
>>> hdmi_match_types[] won't require another if statement to be added to
>>> probe().
>>
>> I don't think it's that easy. of_match_device returns the first match
>> from the device table, so I'd still need to iterate through the
>> matches. I could still break this out into a table, but I don't think
>> of_match_device is the right way to probe it.
>
> You shouldn't have to iterate over multiple matches. of_match_device()
> is supposed to return the match for the first entry in the compatible
> property, then if there was no match, move on to looking at the next
> entry in the compatible property, etc. In practice, I think it's still
> not implemented quite correctly for this, but you can make it work by
> putting the newest compatible value first in the match table.
>
> ___
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
[PATCH 2/2] drm/exynos: Add device tree based discovery support for G2D
On Tue, Feb 5, 2013 at 12:03 PM, Inki Dae wrote: > 2013/2/4 Sachin Kamat : >> On 1 February 2013 18:28, Inki Dae wrote: >>> >>> >>> >>> >>> 2013. 2. 1. ?? 8:52 Inki Dae ??: >>> >>>> >>>> >>>>> -Original Message- >>>>> From: linux-media-owner at vger.kernel.org [mailto:linux-media- >>>>> owner at vger.kernel.org] On Behalf Of Sachin Kamat >>>>> Sent: Friday, February 01, 2013 8:40 PM >>>>> To: Inki Dae >>>>> Cc: Sylwester Nawrocki; Kukjin Kim; Sylwester Nawrocki; linux- >>>>> media at vger.kernel.org; dri-devel at lists.freedesktop.org; devicetree- >>>>> discuss at lists.ozlabs.org; patches at linaro.org >>>>> Subject: Re: [PATCH 2/2] drm/exynos: Add device tree based discovery >>>>> support for G2D >>>>> >>>>> On 1 February 2013 17:02, Inki Dae wrote: >>>>>> >>>>>> How about using like below? >>>>>>Compatible = ""samsung,exynos4x12-fimg-2d" /* for Exynos4212, >>>>>> Exynos4412 */ >>>>>> It looks odd to use "samsung,exynos4212-fimg-2d" saying that this ip is >>>>> for >>>>>> exynos4212 and exynos4412. >>>>> >>>>> AFAIK, compatible strings are not supposed to have any wildcard >>>> characters. >>>>> Compatible string should suggest the first SoC that contained this IP. >>>>> Hence IMO 4212 is OK. >>>>> >>> >>> Oops, one more thing. AFAIK Exynos4210 also has fimg-2d ip. In this case, >>> we should use "samsung,exynos4210-fimg-2d" as comparible string and add it >>> to exynos4210.dtsi? >> >> Exynos4210 has same g2d IP (v3.0) as C110 or V210; so the same >> comptible string will be used for this one too. >> >>> And please check if exynos4212 and 4412 SoCs have same fimg-2d ip. If it's >>> different, we might need to add ip version property or compatible string to >>> each dtsi file to identify the ip version. >> >> AFAIK, they both have the same IP (v4.1). >> > > Ok, let's use the below, > > For exynos4210 SoC, > compatible = "samsung,exynos4210-g2d" > > For exynos4x12 SoCs, > compatible = "samsung,exynos4212-g2d" Even though 4212 is exist, I can't see 4212 board support at mainline. so I prefer exynos4412-g2d instead of 4212-g2d. > > For exynos5250, 5410 (In case of Exynos5440, I'm not sure that the SoC > has same ip) > compatible = "samsung,exynos5250-g2d" Acked-by: Kyungmin Park > > To other guys, > The device tree is used by not only v4l2 side but also drm side so we > should reach an arrangement. So please give me ack if you agree to my > opinion. Otherwise please, give me your opinions. > > Thanks, > Inki Dae > > >>> >>> Sorry but give me your opinions. >>> >>> Thanks, >>> Inki Dae >>> >>> >>>> >>>> Got it. Please post it again. >>>> >>>>> >>>>> -- >>>>> With warm regards, >>>>> Sachin >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-media" in >>>>> the body of a message to majordomo at vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>>> ___ >>>> dri-devel mailing list >>>> dri-devel at lists.freedesktop.org >>>> http://lists.freedesktop.org/mailman/listinfo/dri-devel >> >> >> >> -- >> With warm regards, >> Sachin > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/3] drm/exynos: Get HDMI version from device tree
On Wed, Feb 6, 2013 at 9:42 AM, Stephen Warren swar...@wwwdotorg.org wrote:
On 02/05/2013 05:37 PM, Sean Paul wrote:
On Tue, Feb 5, 2013 at 4:22 PM, Stephen Warren swar...@wwwdotorg.org wrote:
n 02/05/2013 04:42 PM, Sean Paul wrote:
Use the compatible string in the device tree to determine which
registers/functions to use in the HDMI driver. Also changes the
references from v13 to 4210 and v14 to 4212 to reflect the IP
block version instead of the HDMI version.
diff --git a/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
b/Documentation/devicetree/bindings/drm/exynos/hdmi.txt
Binding looks sane to me.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c
b/drivers/gpu/drm/exynos/exynos_hdmi.c
#ifdef CONFIG_OF
static struct of_device_id hdmi_match_types[] = {
{
- .compatible = samsung,exynos5-hdmi,
- .data = (void *)HDMI_TYPE14,
+ .compatible = samsung,exynos4-hdmi,
}, {
/* end node */
}
Why not fill in all the base compatible values there (I think you need
this anyway so that DTs don't all have to be compatible with
samsung,exynos4-hdmi), with .data containing the HDMI_VER_EXYNOS*
values, then ...
At the moment, all DTs have to be compatible with exynos4-hdmi since
it provides the base for the current driver. The driver uses 4210 and
4212 to differentiate between different register addresses and
features, but most things are just exynos4-hdmi compatible.
I would like to distinguish between 4210 and 4x12. since it has
different features. e.g., HDMI v1.3 and v1.4.
and I also want to use 4412 instead of 4212. there's no board to use
4212 at mainline until this time.
Thank you,
Kyungmin Park
The DT nodes should include only the compatible values that the HW is
actually compatible with. If the HW isn't compatible with exynos4-hdmi,
that value shouldn't be in the compatible property, but instead whatever
the base value that the HW really is compatible with. The driver can
support multiple base compatible values from this table.
@@ -2218,17 +2217,18 @@ static int hdmi_probe(struct platform_device *pdev)
+
+ if (of_device_is_compatible(dev-of_node, samsung,exynos4210-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4210;
+ if (of_device_is_compatible(dev-of_node, samsung,exynos4212-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS4212;
+ if (of_device_is_compatible(dev-of_node, samsung,exynos5250-hdmi))
+ hdata-version |= HDMI_VER_EXYNOS5250;
Instead of that, do roughly:
match = of_match_device(hdmi_match_types, pdev-dev);
if (match)
hdata-version |= (int)match-data;
That way, it's all table-based. Any future additions to
hdmi_match_types[] won't require another if statement to be added to
probe().
I don't think it's that easy. of_match_device returns the first match
from the device table, so I'd still need to iterate through the
matches. I could still break this out into a table, but I don't think
of_match_device is the right way to probe it.
You shouldn't have to iterate over multiple matches. of_match_device()
is supposed to return the match for the first entry in the compatible
property, then if there was no match, move on to looking at the next
entry in the compatible property, etc. In practice, I think it's still
not implemented quite correctly for this, but you can make it work by
putting the newest compatible value first in the match table.
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/exynos: Add device tree based discovery support for G2D
On Tue, Feb 5, 2013 at 12:03 PM, Inki Dae inki@samsung.com wrote: 2013/2/4 Sachin Kamat sachin.ka...@linaro.org: On 1 February 2013 18:28, Inki Dae daei...@gmail.com wrote: 2013. 2. 1. 오후 8:52 Inki Dae inki@samsung.com 작성: -Original Message- From: linux-media-ow...@vger.kernel.org [mailto:linux-media- ow...@vger.kernel.org] On Behalf Of Sachin Kamat Sent: Friday, February 01, 2013 8:40 PM To: Inki Dae Cc: Sylwester Nawrocki; Kukjin Kim; Sylwester Nawrocki; linux- me...@vger.kernel.org; dri-devel@lists.freedesktop.org; devicetree- disc...@lists.ozlabs.org; patc...@linaro.org Subject: Re: [PATCH 2/2] drm/exynos: Add device tree based discovery support for G2D On 1 February 2013 17:02, Inki Dae inki@samsung.com wrote: How about using like below? Compatible = samsung,exynos4x12-fimg-2d /* for Exynos4212, Exynos4412 */ It looks odd to use samsung,exynos4212-fimg-2d saying that this ip is for exynos4212 and exynos4412. AFAIK, compatible strings are not supposed to have any wildcard characters. Compatible string should suggest the first SoC that contained this IP. Hence IMO 4212 is OK. Oops, one more thing. AFAIK Exynos4210 also has fimg-2d ip. In this case, we should use samsung,exynos4210-fimg-2d as comparible string and add it to exynos4210.dtsi? Exynos4210 has same g2d IP (v3.0) as C110 or V210; so the same comptible string will be used for this one too. And please check if exynos4212 and 4412 SoCs have same fimg-2d ip. If it's different, we might need to add ip version property or compatible string to each dtsi file to identify the ip version. AFAIK, they both have the same IP (v4.1). Ok, let's use the below, For exynos4210 SoC, compatible = samsung,exynos4210-g2d For exynos4x12 SoCs, compatible = samsung,exynos4212-g2d Even though 4212 is exist, I can't see 4212 board support at mainline. so I prefer exynos4412-g2d instead of 4212-g2d. For exynos5250, 5410 (In case of Exynos5440, I'm not sure that the SoC has same ip) compatible = samsung,exynos5250-g2d Acked-by: Kyungmin Park kyungmin.p...@samsung.com To other guys, The device tree is used by not only v4l2 side but also drm side so we should reach an arrangement. So please give me ack if you agree to my opinion. Otherwise please, give me your opinions. Thanks, Inki Dae Sorry but give me your opinions. Thanks, Inki Dae Got it. Please post it again. -- With warm regards, Sachin -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- With warm regards, Sachin ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: modify burst size based on overlay size
Hi,
On Tue, Nov 20, 2012 at 11:21 PM, Prathyush K
wrote:
> The BURST size of fimd is adjusted based on the number of bytes to be
> read. This is calculated based on the overlay width and the number of
> bits per pixel. There are three burst lengths supported - 4 words,
> 8 words and 16 words where each word is 8 bytes long.
Interesting, BTW can you describe the problem? what's the issue and
why this patch is needed?
does it improve the performance or reduce the fimd bandwidth?
Thank you,
Kyungmin Park
>
> Signed-off-by: Prathyush K
> ---
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 34
> ++
> 1 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 19f12d1..655b2dd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -384,6 +384,7 @@ static void fimd_win_set_pixfmt(struct device *dev,
> unsigned int win)
> struct fimd_context *ctx = get_fimd_context(dev);
> struct fimd_win_data *win_data = >win_data[win];
> unsigned long val;
> + unsigned long bytes;
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> @@ -393,48 +394,63 @@ static void fimd_win_set_pixfmt(struct device *dev,
> unsigned int win)
> case 1:
> val |= WINCON0_BPPMODE_1BPP;
> val |= WINCONx_BITSWP;
> - val |= WINCONx_BURSTLEN_4WORD;
> + bytes = win_data->fb_width >> 3;
> break;
> case 2:
> val |= WINCON0_BPPMODE_2BPP;
> val |= WINCONx_BITSWP;
> - val |= WINCONx_BURSTLEN_8WORD;
> + bytes = win_data->fb_width >> 2;
> break;
> case 4:
> val |= WINCON0_BPPMODE_4BPP;
> val |= WINCONx_BITSWP;
> - val |= WINCONx_BURSTLEN_8WORD;
> + bytes = win_data->fb_width >> 1;
> break;
> case 8:
> val |= WINCON0_BPPMODE_8BPP_PALETTE;
> - val |= WINCONx_BURSTLEN_8WORD;
> val |= WINCONx_BYTSWP;
> + bytes = win_data->fb_width;
> break;
> case 16:
> val |= WINCON0_BPPMODE_16BPP_565;
> val |= WINCONx_HAWSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> + bytes = win_data->fb_width << 1;
> break;
> case 24:
> val |= WINCON0_BPPMODE_24BPP_888;
> val |= WINCONx_WSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> + bytes = win_data->fb_width * 3;
> break;
> case 32:
> val |= WINCON1_BPPMODE_28BPP_A4888
> | WINCON1_BLD_PIX | WINCON1_ALPHA_SEL;
> val |= WINCONx_WSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> + bytes = win_data->fb_width << 2;
> break;
> default:
> DRM_DEBUG_KMS("invalid pixel size so using unpacked
> 24bpp.\n");
> -
> + bytes = win_data->fb_width * 3;
> val |= WINCON0_BPPMODE_24BPP_888;
> val |= WINCONx_WSWP;
> - val |= WINCONx_BURSTLEN_16WORD;
> break;
> }
>
> + /*
> +* Adjust the burst size based on the number of bytes to be read.
> +* Each WORD of the BURST is 8 bytes long. There are 3 BURST sizes
> +* supported by fimd.
> +* WINCONx_BURSTLEN_4WORD = 32 bytes
> +* WINCONx_BURSTLEN_8WORD = 64 bytes
> +* WINCONx_BURSTLEN_16WORD = 128 bytes
> +*/
> + if (bytes < 128)
> + if (bytes < 64)
> + val |= WINCONx_BURSTLEN_4WORD;
> + else
> + val |= WINCONx_BURSTLEN_8WORD;
> + else
> + val |= WINCONx_BURSTLEN_16WORD;
> +
> DRM_DEBUG_KMS("bpp = %d\n", win_data->bpp);
>
> writel(val, ctx->regs + WINCON(win));
> --
> 1.7.0.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames
Hi, To get the proper review, please add proper maintainers. Thank you, Kyungmin Park On Tue, Nov 20, 2012 at 8:55 PM, Rahul Sharma wrote: > Hi All, > > Kindly review the following patch-set. > > regards, > Rahul Sharma > > On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma > wrote: >> This patch set adds provision for composing and sending AVI and AUI >> infoframes by exynos drm hdmi driver. >> >> It also adds provision to get CEA Video ID Code through the display mode >> which is required for making AVI infoframe. >> >> Based on exynos-drm-fixes branch of >> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >> >> Rahul Sharma (1): >> drm: exynos: compose and send avi and aui info frames >> >> Stephane Marchesin (1): >> drm: get cea video id code for a given display mode >> >> drivers/gpu/drm/drm_edid.c | 20 +++ >> drivers/gpu/drm/exynos/exynos_hdmi.c | 97 >> +- >> drivers/gpu/drm/exynos/exynos_hdmi.h | 23 >> drivers/gpu/drm/exynos/regs-hdmi.h | 17 +- >> include/drm/drm_crtc.h |1 + >> 5 files changed, 154 insertions(+), 4 deletions(-) >> > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
On 11/20/12, Prathyush K wrote:
> On Mon, Nov 19, 2012 at 3:14 PM, Kyungmin Park
> wrote:
>
>> Hi,
>>
>> On 11/15/12, Prathyush K wrote:
>> > The 'pages' structure is not required since we can use the 'sgt'. Even
>> for
>> > CONTIG buffers, a SGT is created (which will have just one sgl). This
>> > SGT
>> > can be used during mmap instead of 'pages'. The 'page_size' element of
>> the
>> > structure is also not used anywhere and is removed.
>> > This patch also fixes a memory leak where the 'pages' structure was
>> > being
>> > allocated during gem buffer allocation but not being freed during
>> > deallocate.
>> >
>> > Signed-off-by: Prathyush K
>> > ---
>> > drivers/gpu/drm/exynos/exynos_drm_buf.c| 20 --
>> > drivers/gpu/drm/exynos/exynos_drm_buf.h|4 +-
>> > drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |3 +-
>> > drivers/gpu/drm/exynos/exynos_drm_gem.c| 39
>> > +++
>> > drivers/gpu/drm/exynos/exynos_drm_gem.h|4 ---
>> > 5 files changed, 19 insertions(+), 51 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> > b/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> > index 48c5896..72bf97b 100644
>> > --- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> > +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
>> > @@ -34,8 +34,6 @@ static int lowlevel_buffer_allocate(struct drm_device
>> > *dev,
>> > unsigned int flags, struct exynos_drm_gem_buf *buf)
>> > {
>> > int ret = 0;
>> > - unsigned int npages, i = 0;
>> > - struct scatterlist *sgl;
>> > enum dma_attr attr = DMA_ATTR_FORCE_CONTIGUOUS;
>> >
>> > DRM_DEBUG_KMS("%s\n", __FILE__);
>> > @@ -73,22 +71,6 @@ static int lowlevel_buffer_allocate(struct
>> > drm_device
>> > *dev,
>> > goto err_free_sgt;
>> > }
>> >
>> > - npages = buf->sgt->nents;
>> > -
>> > - buf->pages = kzalloc(sizeof(struct page) * npages, GFP_KERNEL);
>> > - if (!buf->pages) {
>> > - DRM_ERROR("failed to allocate pages.\n");
>> > - ret = -ENOMEM;
>> > - goto err_free_table;
>> > - }
>> > -
>> > - sgl = buf->sgt->sgl;
>> > - while (i < npages) {
>> > - buf->pages[i] = sg_page(sgl);
>> > - sgl = sg_next(sgl);
>> > - i++;
>> > - }
>> > -
>> > DRM_DEBUG_KMS("vaddr(0x%lx), dma_addr(0x%lx), size(0x%lx)\n",
>> > (unsigned long)buf->kvaddr,
>> > (unsigned long)buf->dma_addr,
>> > @@ -96,8 +78,6 @@ static int lowlevel_buffer_allocate(struct drm_device
>> > *dev,
>> >
>> > return ret;
>> >
>> > -err_free_table:
>> > - sg_free_table(buf->sgt);
>> > err_free_sgt:
>> > kfree(buf->sgt);
>> > buf->sgt = NULL;
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h
>> > b/drivers/gpu/drm/exynos/exynos_drm_buf.h
>> > index 3388e4e..25cf162 100644
>> > --- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
>> > +++ b/drivers/gpu/drm/exynos/exynos_drm_buf.h
>> > @@ -34,12 +34,12 @@ struct exynos_drm_gem_buf
>> > *exynos_drm_init_buf(struct
>> > drm_device *dev,
>> > void exynos_drm_fini_buf(struct drm_device *dev,
>> > struct exynos_drm_gem_buf *buffer);
>> >
>> > -/* allocate physical memory region and setup sgt and pages. */
>> > +/* allocate physical memory region and setup sgt. */
>> > int exynos_drm_alloc_buf(struct drm_device *dev,
>> > struct exynos_drm_gem_buf *buf,
>> > unsigned int flags);
>> >
>> > -/* release physical memory region, sgt and pages. */
>> > +/* release physical memory region, and sgt. */
>> > void exynos_drm_free_buf(struct drm_device *dev,
>> > unsigned int flags,
>> > struct exynos_drm_gem_buf *buffer);
>> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>> > b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
>> > index b98da30..615a049 100644
>> > --- a/drive
Re: [PATCH 0/2] drm: exynos: hdmi: sending AVI and AUI info frames
Hi, To get the proper review, please add proper maintainers. Thank you, Kyungmin Park On Tue, Nov 20, 2012 at 8:55 PM, Rahul Sharma r.sh.o...@gmail.com wrote: Hi All, Kindly review the following patch-set. regards, Rahul Sharma On Fri, Nov 9, 2012 at 9:51 PM, Rahul Sharma rahul.sha...@samsung.com wrote: This patch set adds provision for composing and sending AVI and AUI infoframes by exynos drm hdmi driver. It also adds provision to get CEA Video ID Code through the display mode which is required for making AVI infoframe. Based on exynos-drm-fixes branch of git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git Rahul Sharma (1): drm: exynos: compose and send avi and aui info frames Stephane Marchesin (1): drm: get cea video id code for a given display mode drivers/gpu/drm/drm_edid.c | 20 +++ drivers/gpu/drm/exynos/exynos_hdmi.c | 97 +- drivers/gpu/drm/exynos/exynos_hdmi.h | 23 drivers/gpu/drm/exynos/regs-hdmi.h | 17 +- include/drm/drm_crtc.h |1 + 5 files changed, 154 insertions(+), 4 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: modify burst size based on overlay size
Hi, On Tue, Nov 20, 2012 at 11:21 PM, Prathyush K prathyus...@samsung.com wrote: The BURST size of fimd is adjusted based on the number of bytes to be read. This is calculated based on the overlay width and the number of bits per pixel. There are three burst lengths supported - 4 words, 8 words and 16 words where each word is 8 bytes long. Interesting, BTW can you describe the problem? what's the issue and why this patch is needed? does it improve the performance or reduce the fimd bandwidth? Thank you, Kyungmin Park Signed-off-by: Prathyush K prathyus...@samsung.com --- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 34 ++ 1 files changed, 25 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 19f12d1..655b2dd 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -384,6 +384,7 @@ static void fimd_win_set_pixfmt(struct device *dev, unsigned int win) struct fimd_context *ctx = get_fimd_context(dev); struct fimd_win_data *win_data = ctx-win_data[win]; unsigned long val; + unsigned long bytes; DRM_DEBUG_KMS(%s\n, __FILE__); @@ -393,48 +394,63 @@ static void fimd_win_set_pixfmt(struct device *dev, unsigned int win) case 1: val |= WINCON0_BPPMODE_1BPP; val |= WINCONx_BITSWP; - val |= WINCONx_BURSTLEN_4WORD; + bytes = win_data-fb_width 3; break; case 2: val |= WINCON0_BPPMODE_2BPP; val |= WINCONx_BITSWP; - val |= WINCONx_BURSTLEN_8WORD; + bytes = win_data-fb_width 2; break; case 4: val |= WINCON0_BPPMODE_4BPP; val |= WINCONx_BITSWP; - val |= WINCONx_BURSTLEN_8WORD; + bytes = win_data-fb_width 1; break; case 8: val |= WINCON0_BPPMODE_8BPP_PALETTE; - val |= WINCONx_BURSTLEN_8WORD; val |= WINCONx_BYTSWP; + bytes = win_data-fb_width; break; case 16: val |= WINCON0_BPPMODE_16BPP_565; val |= WINCONx_HAWSWP; - val |= WINCONx_BURSTLEN_16WORD; + bytes = win_data-fb_width 1; break; case 24: val |= WINCON0_BPPMODE_24BPP_888; val |= WINCONx_WSWP; - val |= WINCONx_BURSTLEN_16WORD; + bytes = win_data-fb_width * 3; break; case 32: val |= WINCON1_BPPMODE_28BPP_A4888 | WINCON1_BLD_PIX | WINCON1_ALPHA_SEL; val |= WINCONx_WSWP; - val |= WINCONx_BURSTLEN_16WORD; + bytes = win_data-fb_width 2; break; default: DRM_DEBUG_KMS(invalid pixel size so using unpacked 24bpp.\n); - + bytes = win_data-fb_width * 3; val |= WINCON0_BPPMODE_24BPP_888; val |= WINCONx_WSWP; - val |= WINCONx_BURSTLEN_16WORD; break; } + /* +* Adjust the burst size based on the number of bytes to be read. +* Each WORD of the BURST is 8 bytes long. There are 3 BURST sizes +* supported by fimd. +* WINCONx_BURSTLEN_4WORD = 32 bytes +* WINCONx_BURSTLEN_8WORD = 64 bytes +* WINCONx_BURSTLEN_16WORD = 128 bytes +*/ + if (bytes 128) + if (bytes 64) + val |= WINCONx_BURSTLEN_4WORD; + else + val |= WINCONx_BURSTLEN_8WORD; + else + val |= WINCONx_BURSTLEN_16WORD; + DRM_DEBUG_KMS(bpp = %d\n, win_data-bpp); writel(val, ctx-regs + WINCON(win)); -- 1.7.0.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/exynos: add exynos drm specific fb_mmap function
Hi,
On 11/19/12, Prathyush K wrote:
> Changelog v3:
>
> Passing the actual buffer size instead of vm_size to dma_mmap_attrs.
>
> Changelog v2:
>
> Extracting the private data from fb_info structure to obtain the exynos
> gem buffer structure. Now, dma address is obtained from the exynos gem
> buffer structure and not from smem_start. Also calling dma_mmap_attrs
> (instead of dma_mmap_writecombine) with the same attributes used
> during allocation.
>
> Changelog v1:
>
> This patch adds a exynos drm specific implementation of fb_mmap
> which supports mapping a non-contiguous buffer to user space.
>
> This new function does not assume that the frame buffer is contiguous
> and calls dma_mmap_writecombine for mapping the buffer to user space.
> dma_mmap_writecombine will be able to map a contiguous buffer as well
> as non-contig buffer depending on whether an IOMMU mapping is created
> for drm or not.
>
> Signed-off-by: Prathyush K
> ---
> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 30
> +
> 1 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> index 67eb6ba..a6f8cc2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
> @@ -46,8 +46,38 @@ struct exynos_drm_fbdev {
> struct exynos_drm_gem_obj *exynos_gem_obj;
> };
>
> +static int exynos_drm_fb_mmap(struct fb_info *info,
> + struct vm_area_struct *vma)
> +{
> + struct drm_fb_helper *helper = info->par;
> + struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
> + struct exynos_drm_gem_obj *exynos_gem_obj = exynos_fbd->exynos_gem_obj;
> + struct exynos_drm_gem_buf *buffer = exynos_gem_obj->buffer;
> + unsigned long vm_size;
> + int ret;
> +
> + DRM_DEBUG_KMS("%s\n", __func__);
> +
> + vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
How do you sure VM_MIXEDMAP is not required?
> +
> + vm_size = vma->vm_end - vma->vm_start;
> +
> + if (vm_size > buffer->size)
> + return -EINVAL;
does it really happended?
Thank you,
Kyungmin Park
> +
> + ret = dma_mmap_attrs(helper->dev->dev, vma, buffer->kvaddr,
> + buffer->dma_addr, buffer->size, >dma_attrs);
> + if (ret < 0) {
> + DRM_ERROR("failed to mmap.\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> static struct fb_ops exynos_drm_fb_ops = {
> .owner = THIS_MODULE,
> + .fb_mmap= exynos_drm_fb_mmap,
> .fb_fillrect= cfb_fillrect,
> .fb_copyarea= cfb_copyarea,
> .fb_imageblit = cfb_imageblit,
> --
> 1.7.0.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
[PATCH] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
t; -
> - sgl = buf->sgt->sgl;
> - for_each_sg(buf->sgt->sgl, sgl, buf->sgt->nents, i) {
> - if (!sgl) {
> - DRM_ERROR("invalid SG table\n");
> - return -EINTR;
> - }
> - if (page_offset < (sgl->length >> PAGE_SHIFT))
> - break;
> - page_offset -= (sgl->length >> PAGE_SHIFT);
> - }
> -
> - if (i >= buf->sgt->nents) {
> - DRM_ERROR("invalid page offset\n");
> - return -EINVAL;
> - }
> + if (!buf->sgt)
> + return -EINTR;
>
> - pfn = __phys_to_pfn(sg_phys(sgl)) + page_offset;
> - } else {
> - if (!buf->pages)
> + sgl = buf->sgt->sgl;
> + for_each_sg(buf->sgt->sgl, sgl, buf->sgt->nents, i) {
> + if (!sgl) {
It's never happned by for_each_sg definition.
> + DRM_ERROR("invalid SG table\n");
> return -EINTR;
> + }
> + if (page_offset < (sgl->length >> PAGE_SHIFT))
> + break;
> + page_offset -= (sgl->length >> PAGE_SHIFT);
> + }
>
> - pfn = page_to_pfn(buf->pages[0]) + page_offset;
> + if (i >= buf->sgt->nents) {
ditto. IOW it's dead code.
Thank you,
Kyungmin Park
> + DRM_ERROR("invalid page offset\n");
> + return -EINVAL;
> }
>
> + pfn = __phys_to_pfn(sg_phys(sgl)) + page_offset;
> +
> return vm_insert_mixed(vma, f_vaddr, pfn);
> }
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> index 83d21ef..3600b3b 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
> @@ -39,8 +39,6 @@
> * - this address could be physical address without IOMMU and
> * device address with IOMMU.
> * @sgt: sg table to transfer page data.
> - * @pages: contain all pages to allocated memory region.
> - * @page_size: could be 4K, 64K or 1MB.
> * @size: size of allocated memory region.
> */
> struct exynos_drm_gem_buf {
> @@ -48,8 +46,6 @@ struct exynos_drm_gem_buf {
> dma_addr_t dma_addr;
> struct dma_attrsdma_attrs;
> struct sg_table *sgt;
> - struct page **pages;
> - unsigned long page_size;
> unsigned long size;
> };
>
> --
> 1.7.0.4
>
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
Re: [PATCH] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
);
- return -EINVAL;
- }
+ if (!buf-sgt)
+ return -EINTR;
- pfn = __phys_to_pfn(sg_phys(sgl)) + page_offset;
- } else {
- if (!buf-pages)
+ sgl = buf-sgt-sgl;
+ for_each_sg(buf-sgt-sgl, sgl, buf-sgt-nents, i) {
+ if (!sgl) {
It's never happned by for_each_sg definition.
+ DRM_ERROR(invalid SG table\n);
return -EINTR;
+ }
+ if (page_offset (sgl-length PAGE_SHIFT))
+ break;
+ page_offset -= (sgl-length PAGE_SHIFT);
+ }
- pfn = page_to_pfn(buf-pages[0]) + page_offset;
+ if (i = buf-sgt-nents) {
ditto. IOW it's dead code.
Thank you,
Kyungmin Park
+ DRM_ERROR(invalid page offset\n);
+ return -EINVAL;
}
+ pfn = __phys_to_pfn(sg_phys(sgl)) + page_offset;
+
return vm_insert_mixed(vma, f_vaddr, pfn);
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.h
b/drivers/gpu/drm/exynos/exynos_drm_gem.h
index 83d21ef..3600b3b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.h
@@ -39,8 +39,6 @@
* - this address could be physical address without IOMMU and
* device address with IOMMU.
* @sgt: sg table to transfer page data.
- * @pages: contain all pages to allocated memory region.
- * @page_size: could be 4K, 64K or 1MB.
* @size: size of allocated memory region.
*/
struct exynos_drm_gem_buf {
@@ -48,8 +46,6 @@ struct exynos_drm_gem_buf {
dma_addr_t dma_addr;
struct dma_attrsdma_attrs;
struct sg_table *sgt;
- struct page **pages;
- unsigned long page_size;
unsigned long size;
};
--
1.7.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/exynos: add exynos drm specific fb_mmap function
Hi,
On 11/19/12, Prathyush K prathyus...@samsung.com wrote:
Changelog v3:
Passing the actual buffer size instead of vm_size to dma_mmap_attrs.
Changelog v2:
Extracting the private data from fb_info structure to obtain the exynos
gem buffer structure. Now, dma address is obtained from the exynos gem
buffer structure and not from smem_start. Also calling dma_mmap_attrs
(instead of dma_mmap_writecombine) with the same attributes used
during allocation.
Changelog v1:
This patch adds a exynos drm specific implementation of fb_mmap
which supports mapping a non-contiguous buffer to user space.
This new function does not assume that the frame buffer is contiguous
and calls dma_mmap_writecombine for mapping the buffer to user space.
dma_mmap_writecombine will be able to map a contiguous buffer as well
as non-contig buffer depending on whether an IOMMU mapping is created
for drm or not.
Signed-off-by: Prathyush K prathyus...@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 30
+
1 files changed, 30 insertions(+), 0 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
index 67eb6ba..a6f8cc2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c
@@ -46,8 +46,38 @@ struct exynos_drm_fbdev {
struct exynos_drm_gem_obj *exynos_gem_obj;
};
+static int exynos_drm_fb_mmap(struct fb_info *info,
+ struct vm_area_struct *vma)
+{
+ struct drm_fb_helper *helper = info-par;
+ struct exynos_drm_fbdev *exynos_fbd = to_exynos_fbdev(helper);
+ struct exynos_drm_gem_obj *exynos_gem_obj = exynos_fbd-exynos_gem_obj;
+ struct exynos_drm_gem_buf *buffer = exynos_gem_obj-buffer;
+ unsigned long vm_size;
+ int ret;
+
+ DRM_DEBUG_KMS(%s\n, __func__);
+
+ vma-vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
How do you sure VM_MIXEDMAP is not required?
+
+ vm_size = vma-vm_end - vma-vm_start;
+
+ if (vm_size buffer-size)
+ return -EINVAL;
does it really happended?
Thank you,
Kyungmin Park
+
+ ret = dma_mmap_attrs(helper-dev-dev, vma, buffer-kvaddr,
+ buffer-dma_addr, buffer-size, buffer-dma_attrs);
+ if (ret 0) {
+ DRM_ERROR(failed to mmap.\n);
+ return ret;
+ }
+
+ return 0;
+}
+
static struct fb_ops exynos_drm_fb_ops = {
.owner = THIS_MODULE,
+ .fb_mmap= exynos_drm_fb_mmap,
.fb_fillrect= cfb_fillrect,
.fb_copyarea= cfb_copyarea,
.fb_imageblit = cfb_imageblit,
--
1.7.0.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: remove 'pages' and 'page_size' elements in exynos gem buffer
On 11/20/12, Prathyush K prathy...@chromium.org wrote:
On Mon, Nov 19, 2012 at 3:14 PM, Kyungmin Park kmp...@infradead.org
wrote:
Hi,
On 11/15/12, Prathyush K prathyus...@samsung.com wrote:
The 'pages' structure is not required since we can use the 'sgt'. Even
for
CONTIG buffers, a SGT is created (which will have just one sgl). This
SGT
can be used during mmap instead of 'pages'. The 'page_size' element of
the
structure is also not used anywhere and is removed.
This patch also fixes a memory leak where the 'pages' structure was
being
allocated during gem buffer allocation but not being freed during
deallocate.
Signed-off-by: Prathyush K prathyus...@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_buf.c| 20 --
drivers/gpu/drm/exynos/exynos_drm_buf.h|4 +-
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |3 +-
drivers/gpu/drm/exynos/exynos_drm_gem.c| 39
+++
drivers/gpu/drm/exynos/exynos_drm_gem.h|4 ---
5 files changed, 19 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.c
b/drivers/gpu/drm/exynos/exynos_drm_buf.c
index 48c5896..72bf97b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.c
@@ -34,8 +34,6 @@ static int lowlevel_buffer_allocate(struct drm_device
*dev,
unsigned int flags, struct exynos_drm_gem_buf *buf)
{
int ret = 0;
- unsigned int npages, i = 0;
- struct scatterlist *sgl;
enum dma_attr attr = DMA_ATTR_FORCE_CONTIGUOUS;
DRM_DEBUG_KMS(%s\n, __FILE__);
@@ -73,22 +71,6 @@ static int lowlevel_buffer_allocate(struct
drm_device
*dev,
goto err_free_sgt;
}
- npages = buf-sgt-nents;
-
- buf-pages = kzalloc(sizeof(struct page) * npages, GFP_KERNEL);
- if (!buf-pages) {
- DRM_ERROR(failed to allocate pages.\n);
- ret = -ENOMEM;
- goto err_free_table;
- }
-
- sgl = buf-sgt-sgl;
- while (i npages) {
- buf-pages[i] = sg_page(sgl);
- sgl = sg_next(sgl);
- i++;
- }
-
DRM_DEBUG_KMS(vaddr(0x%lx), dma_addr(0x%lx), size(0x%lx)\n,
(unsigned long)buf-kvaddr,
(unsigned long)buf-dma_addr,
@@ -96,8 +78,6 @@ static int lowlevel_buffer_allocate(struct drm_device
*dev,
return ret;
-err_free_table:
- sg_free_table(buf-sgt);
err_free_sgt:
kfree(buf-sgt);
buf-sgt = NULL;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_buf.h
b/drivers/gpu/drm/exynos/exynos_drm_buf.h
index 3388e4e..25cf162 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_buf.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_buf.h
@@ -34,12 +34,12 @@ struct exynos_drm_gem_buf
*exynos_drm_init_buf(struct
drm_device *dev,
void exynos_drm_fini_buf(struct drm_device *dev,
struct exynos_drm_gem_buf *buffer);
-/* allocate physical memory region and setup sgt and pages. */
+/* allocate physical memory region and setup sgt. */
int exynos_drm_alloc_buf(struct drm_device *dev,
struct exynos_drm_gem_buf *buf,
unsigned int flags);
-/* release physical memory region, sgt and pages. */
+/* release physical memory region, and sgt. */
void exynos_drm_free_buf(struct drm_device *dev,
unsigned int flags,
struct exynos_drm_gem_buf *buffer);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index b98da30..615a049 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -93,8 +93,7 @@ static struct sg_table *
goto err_unlock;
}
- DRM_DEBUG_PRIME(buffer size = 0x%lx page_size = 0x%lx\n,
- buf-size, buf-page_size);
+ DRM_DEBUG_PRIME(buffer size = 0x%lx\n, buf-size);
err_unlock:
mutex_unlock(dev-struct_mutex);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c
b/drivers/gpu/drm/exynos/exynos_drm_gem.c
index 50d73f1..40999ac 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_gem.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c
@@ -99,34 +99,27 @@ static int exynos_drm_gem_map_buf(struct
drm_gem_object
*obj,
unsigned long pfn;
int i;
- if (exynos_gem_obj-flags EXYNOS_BO_NONCONTIG) {
- if (!buf-sgt)
- return -EINTR;
-
- sgl = buf-sgt-sgl;
- for_each_sg(buf-sgt-sgl, sgl, buf-sgt-nents, i) {
- if (!sgl) {
- DRM_ERROR(invalid SG table\n);
- return -EINTR;
- }
- if (page_offset
[Linaro-mm-sig] [PATCHv9 00/25] Integration of videobuf2 with DMABUF
On 10/10/12, Mauro Carvalho Chehab wrote: > Hi, > > Em Tue, 02 Oct 2012 16:27:11 +0200 > Tomasz Stanislawski escreveu: > >> Hello everyone, >> This patchset adds support for DMABUF [2] importing and exporting to V4L2 >> stack. >> >> v9: >> - rebase on 3.6 >> - change type for fs to __s32 >> - add support for vb2_ioctl_expbuf >> - remove patch 'v4l: vb2: add support for DMA_ATTR_NO_KERNEL_MAPPING', >> it will be posted as a separate patch >> - fix typos and style in Documentation (from Hans Verkuil) >> - only vb2-core and vb2-dma-contig selects DMA_SHARED_BUFFER in Kconfig >> - use data_offset and length while queueing DMABUF >> - return the most recently used fd at VIDIOC_DQBUF >> - use (buffer-type, index, plane) tuple instead of mem_offset >> to identify a for exporting (from Hans Verkuil) >> - support for DMABUF mmap in vb2-dma-contig (from Laurent Pinchart) >> - add testing alignment of vaddr and size while verifying userptr >> against DMA capabilities (from Laurent Pinchart) >> - substitute VM_DONTDUMP with VM_RESERVED >> - simplify error handling in vb2_dc_get_dmabuf (from Laurent Pinchart) > > For now, NACK. See below. Sad news! It's failed to merge by very poor samsung board support at mainline. CC arm & samsung mailing list. Thank you, Kyungmin Park > > I spent 4 days trying to setup an environment that would allow testing > DMABUF with video4linux without success (long story, see below). Also, > Laurent tried the same without any luck, and it seems that it even > corrupted his test machine. > > Basically Samsung generously donated me two boards that it could be > used on this test (Origen and SMDK310). None of them actually worked > with the upstream kernel: patches are needed to avoid OOPSes on > Origen; both Origen/SMDK310 defconfigs are completely broken, and drivers > don't even boot if someone tries to use the Kernel's defconfigs. > > Even after spending _days_ trying to figure out the needed .config options > (as the config files are not easily available), both boards have... issues: > > - lack of any display output driver at SMDK310; > > - lack of any network driver at Origen: it seems that none of > the available network options on Origen was upstreamed: no Bluetooth, no > OTG, > no Wifi. > > The only test I was able to do (yesterday, late night), the DMABUF caused > an OOPS at the Origen board. So, for sure it is not ready for upstream. > > It is now, too late for 3.7. I might consider it to 3.8, if something > can be done to fix the existing issues, and setup a proper setup, using > the upstream Kernel. > > Regards, > Mauro > > ___ > Linaro-mm-sig mailing list > Linaro-mm-sig at lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-mm-sig >
Re: [Linaro-mm-sig] [PATCHv9 00/25] Integration of videobuf2 with DMABUF
On 10/10/12, Mauro Carvalho Chehab mche...@redhat.com wrote: Hi, Em Tue, 02 Oct 2012 16:27:11 +0200 Tomasz Stanislawski t.stanisl...@samsung.com escreveu: Hello everyone, This patchset adds support for DMABUF [2] importing and exporting to V4L2 stack. v9: - rebase on 3.6 - change type for fs to __s32 - add support for vb2_ioctl_expbuf - remove patch 'v4l: vb2: add support for DMA_ATTR_NO_KERNEL_MAPPING', it will be posted as a separate patch - fix typos and style in Documentation (from Hans Verkuil) - only vb2-core and vb2-dma-contig selects DMA_SHARED_BUFFER in Kconfig - use data_offset and length while queueing DMABUF - return the most recently used fd at VIDIOC_DQBUF - use (buffer-type, index, plane) tuple instead of mem_offset to identify a for exporting (from Hans Verkuil) - support for DMABUF mmap in vb2-dma-contig (from Laurent Pinchart) - add testing alignment of vaddr and size while verifying userptr against DMA capabilities (from Laurent Pinchart) - substitute VM_DONTDUMP with VM_RESERVED - simplify error handling in vb2_dc_get_dmabuf (from Laurent Pinchart) For now, NACK. See below. Sad news! It's failed to merge by very poor samsung board support at mainline. CC arm samsung mailing list. Thank you, Kyungmin Park I spent 4 days trying to setup an environment that would allow testing DMABUF with video4linux without success (long story, see below). Also, Laurent tried the same without any luck, and it seems that it even corrupted his test machine. Basically Samsung generously donated me two boards that it could be used on this test (Origen and SMDK310). None of them actually worked with the upstream kernel: patches are needed to avoid OOPSes on Origen; both Origen/SMDK310 defconfigs are completely broken, and drivers don't even boot if someone tries to use the Kernel's defconfigs. Even after spending _days_ trying to figure out the needed .config options (as the config files are not easily available), both boards have... issues: - lack of any display output driver at SMDK310; - lack of any network driver at Origen: it seems that none of the available network options on Origen was upstreamed: no Bluetooth, no OTG, no Wifi. The only test I was able to do (yesterday, late night), the DMABUF caused an OOPS at the Origen board. So, for sure it is not ready for upstream. It is now, too late for 3.7. I might consider it to 3.8, if something can be done to fix the existing issues, and setup a proper setup, using the upstream Kernel. Regards, Mauro ___ Linaro-mm-sig mailing list linaro-mm-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-mm-sig ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/14] drm: exynos: hdmi: add dt based support for exynos5 hdmi
To Mr. Shim, Can you check these patch series? Thank you, Kyungmin Park On 9/28/12, Rahul Sharma wrote: > This patch set adds the DT based support for Samsung's Exynos5250 in > DRM-HDMI. > It includes disabling of hdmi internal interrupt, suppport for platform > variants for hdmi and mixer, support to disable video processor based on > platform type and removal of drm common platform data. > > Rahul Sharma (9): > drm: exynos: remove drm hdmi platform data struct > drm: exynos: hdmi: add support for exynos5 ddc > drm: exynos: hdmi: add support for exynos5 hdmiphy > drm: exynos: hdmi: add support for platform variants for mixer > drm: exynos: hdmi: add support to disable video processor in mixer > drm: exynos: hdmi: add support for exynos5 mixer > drm: exynos: hdmi: replace is_v13 with version check in hdmi > drm: exynos: hdmi: add support for exynos5 hdmi > drm: exynos: hdmi: remove drm common hdmi platform data struct > > Tomasz Stanislawski (5): > media: s5p-hdmi: add HPD GPIO to platform data > drm: exynos: hdmi: support for platform variants > drm: exynos: hdmi: fix interrupt handling > drm: exynos: hdmi: use s5p-hdmi platform data > drm: exynos: hdmi: turn off HPD interrupt in HDMI chip > > drivers/gpu/drm/exynos/exynos_ddc.c | 22 +++- > drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 51 > drivers/gpu/drm/exynos/exynos_drm_hdmi.h |2 + > drivers/gpu/drm/exynos/exynos_hdmi.c | 196 > --- > drivers/gpu/drm/exynos/exynos_hdmiphy.c | 12 ++- > drivers/gpu/drm/exynos/exynos_mixer.c| 219 > ++ > drivers/gpu/drm/exynos/regs-mixer.h |2 + > include/drm/exynos_drm.h | 27 > include/media/s5p_hdmi.h |2 + > 9 files changed, 369 insertions(+), 164 deletions(-) > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
[PATCH V4 2/2] video: drm: exynos: Add device tree support
Hi,
On Thu, Sep 6, 2012 at 12:39 AM, Leela Krishna Amudala
wrote:
> Add device tree based discovery support for DRM-FIMD driver.
>
> Signed-off-by: Leela Krishna Amudala
> ---
> Documentation/devicetree/bindings/fb/drm-fimd.txt | 80 +
> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 95
> -
> 2 files changed, 173 insertions(+), 2 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
>
> diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt
> b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> new file mode 100644
> index 000..4ff1829
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
> @@ -0,0 +1,80 @@
> +* Samsung Display Controller using DRM frame work
> +
> +The display controller is used to transfer image data from memory to an
> +external LCD driver interface. It supports various color formats such as
> +rgb and yuv.
> +
> +Required properties:
> + - compatible: Should be "samsung,exynos5-fimd" or "samsung,exynos4-fb" for
Doesn't better to use single word? fimd or fb?. I think 'fb' is used
for framebuffer historically.
but now it's used both fb and drm, so fimd is neutral and architecture
specific.
To do this, Modify arch-exynos first and update it at each drivers it properly.
Thank you,
Kyungmin Park
> + fimd using DRM frame work.
> + - reg: physical base address of the controller and length of memory
> + mapped region.
> + - interrupts: Three interrupts should be specified. The interrupts should be
> + specified in the following order.
> + - VSYNC interrupt
> + - FIFO level interrupt
> + - FIMD System Interrupt
> +
> + - samsung,fimd-display: This property should specify the phandle of the
> + display device node which holds the video interface timing with the
> + below mentioned properties.
> +
> + - lcd-htiming: Specifies the horizontal timing for the overlay. The
> + horizontal timing includes four parameters in the following order.
> +
> + - horizontal back porch (in number of lcd clocks)
> + - horizontal front porch (in number of lcd clocks)
> + - hsync pulse width (in number of lcd clocks)
> + - Display panels X resolution.
> +
> + - lcd-vtiming: Specifies the vertical timing for the overlay. The
> + vertical timing includes four parameters in the following order.
> +
> + - vertical back porch (in number of lcd lines)
> + - vertical front porch (in number of lcd lines)
> + - vsync pulse width (in number of lcd clocks)
> + - Display panels Y resolution.
> +
> +
> + - samsung,default-window: Specifies the default window number of the fimd
> controller.
> +
> + - samsung,fimd-win-bpp: Specifies the bits per pixel.
> +
> +Optional properties:
> + - samsung,fimd-vidout-rgb: Video output format is RGB.
> + - samsung,fimd-inv-vclk: invert video clock polarity.
> + - samsung,fimd-frame-rate: Number of video frames per second.
> +
> +Example:
> +
> + The following is an example for the fimd controller is split into
> + two portions. The SoC specific portion can be specified in the SoC
> + specific dts file. The board specific portion can be specified in the
> + board specific dts file.
> +
> + - SoC Specific portion
> +
> + fimd {
> + compatible = "samsung,exynos5-fimd";
> + interrupt-parent = <>;
> + reg = <0x1440 0x4>;
> + interrupts = <18 5>, <18 4>, <18 6>;
> + };
> +
> + - Board Specific portion
> +
> + lcd_fimd0: lcd_panel0 {
> + lcd-htiming = <4 4 4 480>;
> + lcd-vtiming = <4 4 4 320>;
> + supports-mipi-panel;
> + };
> +
> + fimd {
> + samsung,fimd-display = <_fimd0>;
> + samsung,fimd-vidout-rgb;
> + samsung,fimd-inv-vclk;
> + samsung,fimd-frame-rate = <60>;
> + samsung,default-window = <0>;
> + samsung,fimd-win-bpp = <32>;
> + };
> +
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 3701fbe..a4fa8e9 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -103,9 +104,18 @@ struct fimd_context {
> struct exynos_drm_panel_info *panel;
> };
>
> +static const
Re: [PATCH V4 2/2] video: drm: exynos: Add device tree support
Hi,
On Thu, Sep 6, 2012 at 12:39 AM, Leela Krishna Amudala
l.kris...@samsung.com wrote:
Add device tree based discovery support for DRM-FIMD driver.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
---
Documentation/devicetree/bindings/fb/drm-fimd.txt | 80 +
drivers/gpu/drm/exynos/exynos_drm_fimd.c | 95
-
2 files changed, 173 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/fb/drm-fimd.txt
diff --git a/Documentation/devicetree/bindings/fb/drm-fimd.txt
b/Documentation/devicetree/bindings/fb/drm-fimd.txt
new file mode 100644
index 000..4ff1829
--- /dev/null
+++ b/Documentation/devicetree/bindings/fb/drm-fimd.txt
@@ -0,0 +1,80 @@
+* Samsung Display Controller using DRM frame work
+
+The display controller is used to transfer image data from memory to an
+external LCD driver interface. It supports various color formats such as
+rgb and yuv.
+
+Required properties:
+ - compatible: Should be samsung,exynos5-fimd or samsung,exynos4-fb for
Doesn't better to use single word? fimd or fb?. I think 'fb' is used
for framebuffer historically.
but now it's used both fb and drm, so fimd is neutral and architecture
specific.
To do this, Modify arch-exynos first and update it at each drivers it properly.
Thank you,
Kyungmin Park
+ fimd using DRM frame work.
+ - reg: physical base address of the controller and length of memory
+ mapped region.
+ - interrupts: Three interrupts should be specified. The interrupts should be
+ specified in the following order.
+ - VSYNC interrupt
+ - FIFO level interrupt
+ - FIMD System Interrupt
+
+ - samsung,fimd-display: This property should specify the phandle of the
+ display device node which holds the video interface timing with the
+ below mentioned properties.
+
+ - lcd-htiming: Specifies the horizontal timing for the overlay. The
+ horizontal timing includes four parameters in the following order.
+
+ - horizontal back porch (in number of lcd clocks)
+ - horizontal front porch (in number of lcd clocks)
+ - hsync pulse width (in number of lcd clocks)
+ - Display panels X resolution.
+
+ - lcd-vtiming: Specifies the vertical timing for the overlay. The
+ vertical timing includes four parameters in the following order.
+
+ - vertical back porch (in number of lcd lines)
+ - vertical front porch (in number of lcd lines)
+ - vsync pulse width (in number of lcd clocks)
+ - Display panels Y resolution.
+
+
+ - samsung,default-window: Specifies the default window number of the fimd
controller.
+
+ - samsung,fimd-win-bpp: Specifies the bits per pixel.
+
+Optional properties:
+ - samsung,fimd-vidout-rgb: Video output format is RGB.
+ - samsung,fimd-inv-vclk: invert video clock polarity.
+ - samsung,fimd-frame-rate: Number of video frames per second.
+
+Example:
+
+ The following is an example for the fimd controller is split into
+ two portions. The SoC specific portion can be specified in the SoC
+ specific dts file. The board specific portion can be specified in the
+ board specific dts file.
+
+ - SoC Specific portion
+
+ fimd {
+ compatible = samsung,exynos5-fimd;
+ interrupt-parent = combiner;
+ reg = 0x1440 0x4;
+ interrupts = 18 5, 18 4, 18 6;
+ };
+
+ - Board Specific portion
+
+ lcd_fimd0: lcd_panel0 {
+ lcd-htiming = 4 4 4 480;
+ lcd-vtiming = 4 4 4 320;
+ supports-mipi-panel;
+ };
+
+ fimd {
+ samsung,fimd-display = lcd_fimd0;
+ samsung,fimd-vidout-rgb;
+ samsung,fimd-inv-vclk;
+ samsung,fimd-frame-rate = 60;
+ samsung,default-window = 0;
+ samsung,fimd-win-bpp = 32;
+ };
+
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 3701fbe..a4fa8e9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -18,6 +18,7 @@
#include linux/platform_device.h
#include linux/clk.h
#include linux/pm_runtime.h
+#include linux/of.h
#include video/samsung_fimd.h
#include drm/exynos_drm.h
@@ -103,9 +104,18 @@ struct fimd_context {
struct exynos_drm_panel_info *panel;
};
+static const struct of_device_id fimd_dt_match[];
+
static inline struct fimd_driver_data *drm_fimd_get_driver_data(
struct platform_device *pdev)
{
+#ifdef CONFIG_OF
+ if (pdev-dev.of_node) {
+ const struct of_device_id *match;
+ match = of_match_ptr(fimd_dt_match);
+ return (struct fimd_driver_data *)match-data;
+ }
+#endif
return (struct fimd_driver_data
exynos drm hdmi audio: how to recieve audio parameters (sf, bps, channel count)
Hi, I bet 1st option. And configure it at user space. IOW set the proper ALSA kcontrol. And set the proper DRM audio config by IOCTL.. Any other opinions? Thank you, Kyungmin Park From: RAHUL SHARMA [mailto:rahul.sha...@samsung.com] Sent: Thursday, July 19, 2012 6:57 PM To: dri-devel at lists.freedesktop.org; SUNIL JOSHI; PRASHANTH GODREHAL; Prathyush Kalashwaram; PADMAVATHI VENNA; CHANDRASEKAR RAMAKRISHNAN; SHIRISH S; FAHAD KUNNATHADI; SUNIL M; Joonyoung Shim; Seung-Woo Kim; inki.dae at samsung.com; kyungmin.park at samsung.com Subject: Fwd: exynos drm hdmi audio: how to recieve audio parameters (sf, bps, channel count) Hi Inki, Please suggest me a solution for this problem. regards, Rahul Sharma --- Original Message --- Sender : RAHUL SHARMA ./Technical Manager/SISO-Solution 1/Samsung Electronics Date : Jul 17, 2012 11:42 (GMT+05:30) Title : exynos drm hdmi audio: how to recieve audio parameters (sf, bps, channel count) hi, I am adding support for hdmi audio, inside exynos drm hdmi driver. Hdmi audio is initialized with default parameters. I want to implement the mechanism to update hdmi registers, whenever audio properties got changed (i2s/spdif). raedon/r600 drm dirver is polling the audio ip every 100 ms and reconfigure the hdmi audio block. This is not possible with exynos as all information cannot be collected from i2s tx registers. It is directly set on wm8994 connected through i2c. Possible solution: 1) drm driver exposing ioctl for setting audio parameters. 2) alsa driver notifying the change in audio parameters through kernel notifiers. drm hdmi driver subscribed for the same. Please suggest me a good way to implement this. regards, Rahul Sharma. -- next part -- An HTML attachment was scrubbed... URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120720/6e797d74/attachment-0001.html> -- next part -- A non-text attachment was scrubbed... Name: image001.jpg Type: image/jpeg Size: 72722 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/dri-devel/attachments/20120720/6e797d74/attachment-0001.jpg>
[PATCH 1/2] video: drm: exynos: Add device tree support
Hi,
On Fri, Jul 6, 2012 at 9:28 PM, Leela Krishna Amudala
wrote:
> Add device tree based discovery support for DRM-FIMD driver.
>
> Signed-off-by: Leela Krishna Amudala
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 29fdbfe..37769cf 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -18,6 +18,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -788,12 +789,84 @@ static int fimd_power_on(struct fimd_context *ctx, bool
> enable)
> return 0;
> }
>
> +#ifdef CONFIG_OF
> +static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device
> *dev)
> +{
> + struct device_node *np = dev->of_node;
> + struct device_node *disp_np;
> + struct exynos_drm_fimd_pdata *pd;
> + u32 data[4];
> +
> + pd = kzalloc(sizeof(*pd), GFP_KERNEL);
> + if (!pd) {
> + dev_err(dev, "memory allocation for pdata failed\n");
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + if (of_get_property(np, "samsung,fimd-vidout-rgb", NULL))
> + pd->vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
> + if (of_get_property(np, "samsung,fimd-vidout-tv", NULL))
> + pd->vidcon0 |= VIDCON0_VIDOUT_TV;
> + if (of_get_property(np, "samsung,fimd-inv-hsync", NULL))
> + pd->vidcon1 |= VIDCON1_INV_HSYNC;
> + if (of_get_property(np, "samsung,fimd-inv-vsync", NULL))
> + pd->vidcon1 |= VIDCON1_INV_VSYNC;
> + if (of_get_property(np, "samsung,fimd-inv-vclk", NULL))
> + pd->vidcon1 |= VIDCON1_INV_VCLK;
> + if (of_get_property(np, "samsung,fimd-inv-vden", NULL))
> + pd->vidcon1 |= VIDCON1_INV_VDEN;
> +
> + disp_np = of_parse_phandle(np, "samsung,fimd-display", 0);
> + if (!disp_np) {
> + dev_err(dev, "unable to find display panel info\n");
> + return ERR_PTR(-EINVAL);
> + }
> +
> + if (of_property_read_u32_array(disp_np, "lcd-htiming", data, 4)) {
> + dev_err(dev, "invalid horizontal timing\n");
> + return ERR_PTR(-EINVAL);
> + }
> + pd->panel.timing.left_margin = data[0];
> + pd->panel.timing.right_margin = data[1];
> + pd->panel.timing.hsync_len = data[2];
> + pd->panel.timing.xres = data[3];
> +
> + if (of_property_read_u32_array(disp_np, "lcd-vtiming", data, 4)) {
> + dev_err(dev, "invalid vertical timing\n");
> + return ERR_PTR(-EINVAL);
> + }
> + pd->panel.timing.upper_margin = data[0];
> + pd->panel.timing.lower_margin = data[1];
> + pd->panel.timing.vsync_len = data[2];
> + pd->panel.timing.yres = data[3];
> +
> + of_property_read_u32(disp_np, "lcd-panel-type", >panel_type);
> +
> + of_property_read_u32(np, "samsung,fimd-frame-rate",
> + >panel.timing.refresh);
> +
> + of_property_read_u32(np, "samsung, defalut-window", >default_win);
No space between after comma.
> +
> + of_property_read_u32(np, "samsung,fimd-win-bpp", >bpp);
> +
> + return pd;
> +}
> +#else
> +static int drm_fimd_dt_parse_pdata(struct device *dev,
> + struct exynos_drm_fimd_pdata **pdata)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_OF */
> +
> +static const struct of_device_id drm_fimd_dt_match[];
> +
> static int __devinit fimd_probe(struct platform_device *pdev)
> {
> struct device *dev = >dev;
> struct fimd_context *ctx;
> struct exynos_drm_subdrv *subdrv;
> - struct exynos_drm_fimd_pdata *pdata;
> + struct exynos_drm_fimd_pdata *pdata = pdev->dev.platform_data;
> struct exynos_drm_panel_info *panel;
> struct resource *res;
> int win;
> @@ -801,7 +874,11 @@ static int __devinit fimd_probe(struct platform_device
> *pdev)
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> - pdata = pdev->dev.platform_data;
> + if (pdev->dev.of_node) {
> + pdata = drm_fimd_dt_parse_pdata(>dev);
> + if (IS_ERR(pdata))
> + return PTR_ERR(pdata);
> + }
> if (!pdata) {
> dev_err(dev, "no platform data specified\n");
>
Re: [PATCH 1/2] video: drm: exynos: Add device tree support
Hi,
On Fri, Jul 6, 2012 at 9:28 PM, Leela Krishna Amudala
l.kris...@samsung.com wrote:
Add device tree based discovery support for DRM-FIMD driver.
Signed-off-by: Leela Krishna Amudala l.kris...@samsung.com
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 29fdbfe..37769cf 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -18,6 +18,7 @@
#include linux/platform_device.h
#include linux/clk.h
#include linux/pm_runtime.h
+#include linux/of.h
#include drm/exynos_drm.h
#include plat/regs-fb-v4.h
@@ -788,12 +789,84 @@ static int fimd_power_on(struct fimd_context *ctx, bool
enable)
return 0;
}
+#ifdef CONFIG_OF
+static struct exynos_drm_fimd_pdata *drm_fimd_dt_parse_pdata(struct device
*dev)
+{
+ struct device_node *np = dev-of_node;
+ struct device_node *disp_np;
+ struct exynos_drm_fimd_pdata *pd;
+ u32 data[4];
+
+ pd = kzalloc(sizeof(*pd), GFP_KERNEL);
+ if (!pd) {
+ dev_err(dev, memory allocation for pdata failed\n);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ if (of_get_property(np, samsung,fimd-vidout-rgb, NULL))
+ pd-vidcon0 |= VIDCON0_VIDOUT_RGB | VIDCON0_PNRMODE_RGB;
+ if (of_get_property(np, samsung,fimd-vidout-tv, NULL))
+ pd-vidcon0 |= VIDCON0_VIDOUT_TV;
+ if (of_get_property(np, samsung,fimd-inv-hsync, NULL))
+ pd-vidcon1 |= VIDCON1_INV_HSYNC;
+ if (of_get_property(np, samsung,fimd-inv-vsync, NULL))
+ pd-vidcon1 |= VIDCON1_INV_VSYNC;
+ if (of_get_property(np, samsung,fimd-inv-vclk, NULL))
+ pd-vidcon1 |= VIDCON1_INV_VCLK;
+ if (of_get_property(np, samsung,fimd-inv-vden, NULL))
+ pd-vidcon1 |= VIDCON1_INV_VDEN;
+
+ disp_np = of_parse_phandle(np, samsung,fimd-display, 0);
+ if (!disp_np) {
+ dev_err(dev, unable to find display panel info\n);
+ return ERR_PTR(-EINVAL);
+ }
+
+ if (of_property_read_u32_array(disp_np, lcd-htiming, data, 4)) {
+ dev_err(dev, invalid horizontal timing\n);
+ return ERR_PTR(-EINVAL);
+ }
+ pd-panel.timing.left_margin = data[0];
+ pd-panel.timing.right_margin = data[1];
+ pd-panel.timing.hsync_len = data[2];
+ pd-panel.timing.xres = data[3];
+
+ if (of_property_read_u32_array(disp_np, lcd-vtiming, data, 4)) {
+ dev_err(dev, invalid vertical timing\n);
+ return ERR_PTR(-EINVAL);
+ }
+ pd-panel.timing.upper_margin = data[0];
+ pd-panel.timing.lower_margin = data[1];
+ pd-panel.timing.vsync_len = data[2];
+ pd-panel.timing.yres = data[3];
+
+ of_property_read_u32(disp_np, lcd-panel-type, pd-panel_type);
+
+ of_property_read_u32(np, samsung,fimd-frame-rate,
+ pd-panel.timing.refresh);
+
+ of_property_read_u32(np, samsung, defalut-window, pd-default_win);
No space between after comma.
+
+ of_property_read_u32(np, samsung,fimd-win-bpp, pd-bpp);
+
+ return pd;
+}
+#else
+static int drm_fimd_dt_parse_pdata(struct device *dev,
+ struct exynos_drm_fimd_pdata **pdata)
+{
+ return 0;
+}
+#endif /* CONFIG_OF */
+
+static const struct of_device_id drm_fimd_dt_match[];
+
static int __devinit fimd_probe(struct platform_device *pdev)
{
struct device *dev = pdev-dev;
struct fimd_context *ctx;
struct exynos_drm_subdrv *subdrv;
- struct exynos_drm_fimd_pdata *pdata;
+ struct exynos_drm_fimd_pdata *pdata = pdev-dev.platform_data;
struct exynos_drm_panel_info *panel;
struct resource *res;
int win;
@@ -801,7 +874,11 @@ static int __devinit fimd_probe(struct platform_device
*pdev)
DRM_DEBUG_KMS(%s\n, __FILE__);
- pdata = pdev-dev.platform_data;
+ if (pdev-dev.of_node) {
+ pdata = drm_fimd_dt_parse_pdata(pdev-dev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }
if (!pdata) {
dev_err(dev, no platform data specified\n);
return -EINVAL;
@@ -1006,6 +1083,15 @@ static int fimd_runtime_resume(struct device *dev)
}
#endif
+#ifdef CONFIG_OF
+static const struct of_device_id drm_fimd_dt_match[] = {
+ { .compatible = samsung,exynos5-fb,
It's ambiguous. it's better to use samsung,exynos5-drm.
Yes I know, previous it uses exynos4-fb to reduce the modification
with mainline. but correct name is exynoxX-drm.
Thank you,
Kyungmin Park
+ .data = (void *)NULL },
+ {},
+};
+MODULE_DEVICE_TABLE(of, drm_fimd_dt_match);
+#endif
+
static const struct dev_pm_ops fimd_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS
[PATCH 2/2 v3] drm/exynos: added userptr feature.
On 5/10/12, Minchan Kim wrote: > On 05/10/2012 03:53 PM, KOSAKI Motohiro wrote: > >> (5/10/12 12:58 AM), Minchan Kim wrote: >>> On 05/10/2012 10:39 AM, Inki Dae wrote: >>> >>>> Hi Jerome, >>>> >>>>> -Original Message- >>>>> From: Jerome Glisse [mailto:j.glisse at gmail.com] >>>>> Sent: Wednesday, May 09, 2012 11:46 PM >>>>> To: Inki Dae >>>>> Cc: airlied at linux.ie; dri-devel at lists.freedesktop.org; >>>>> kyungmin.park at samsung.com; sw0312.kim at samsung.com; linux-mm at >>>>> kvack.org >>>>> Subject: Re: [PATCH 2/2 v3] drm/exynos: added userptr feature. >>>>> >>>>> On Wed, May 9, 2012 at 2:17 AM, Inki Dae wrote: >>>>>> this feature is used to import user space region allocated by >>>>>> malloc() >>>>> or >>>>>> mmaped into a gem. and to guarantee the pages to user space not to be >>>>>> swapped out, the VMAs within the user space would be locked and then >>>>> unlocked >>>>>> when the pages are released. >>>>>> >>>>>> but this lock might result in significant degradation of system >>>>> performance >>>>>> because the pages couldn't be swapped out so we limit user-desired >>>>> userptr >>>>>> size to pre-defined. >>>>>> >>>>>> Signed-off-by: Inki Dae >>>>>> Signed-off-by: Kyungmin Park >>>>> >>>>> >>>>> Again i would like feedback from mm people (adding cc). I am not sure >>>> >>>> Thank you, I missed adding mm as cc. >>>> >>>>> locking the vma is the right anwser as i said in my previous mail, >>>>> userspace can munlock it in your back, maybe VM_RESERVED is better. >>>> >>>> I know that with VM_RESERVED flag, also we can avoid the pages from >>>> being >>>> swapped out. but these pages should be unlocked anytime we want >>>> because we >>>> could allocate all pages on system and lock them, which in turn, it may >>>> result in significant deterioration of system performance.(maybe other >>>> processes requesting free memory would be blocked) so I used >>>> VM_LOCKED flags >>>> instead. but I'm not sure this way is best also. >>>> >>>>> Anyway even not considering that you don't check at all that process >>>>> don't go over the limit of locked page see mm/mlock.c RLIMIT_MEMLOCK >>>> >>>> Thank you for your advices. >>>> >>>>> for how it's done. Also you mlock complete vma but the userptr you get >>>>> might be inside say 16M vma and you only care about 1M of userptr, if >>>>> you mark the whole vma as locked than anytime a new page is fault in >>>>> the vma else where than in the buffer you are interested then it got >>>>> allocated for ever until the gem buffer is destroy, i am not sure of >>>>> what happen to the vma on next malloc if it grows or not (i would >>>>> think it won't grow at it would have different flags than new >>>>> anonymous memory). >>> >>> >>> I don't know history in detail because you didn't have sent full >>> patches to linux-mm and >>> I didn't read the below code, either. >>> Just read your description and reply of Jerome. Apparently, there is >>> something I missed. >>> >>> Your goal is to avoid swap out some user pages which is used in kernel >>> at the same time. Right? >>> Let's use get_user_pages. Is there any issue you can't use it? >> >> Maybe because get_user_pages() is fork unsafe? dunno. > > > If there is such problem, I think user program should handle it by > MADV_DONTFORK > and make to allow write by only parent process. Please read the original patches and discuss the root cause. Does it harm to pass user space memory to kernel space and how to make is possible at DRM? Thank you, Kyungmin Park > > -- > Kind regards, > Minchan Kim > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo at kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Fight unfair telecom internet charges in Canada: sign > http://stopthemeter.ca/ > Don't email: mailto:"dont at kvack.org"> email at kvack.org >
[PATCH 1/4] [RFC] drm/exynos: DMABUF: Added support for exporting non-contig buffers
Hi,
I saw your description but please generate the patches against the
latest codes at next time.
On 4/14/12, Prathyush wrote:
> With this change, the exynos drm dmabuf module can export and
> import dmabuf of gem objects with non-continuous memory.
>
> The exynos_map_dmabuf function can create SGT of a non-contiguous
> buffer by calling dma_get_pages to retrieve the allocated pages
> and then maps the SGT to the caller's address space.
>
> Signed-off-by: Prathyush K
> ---
> drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 98
> +++-
> 1 files changed, 81 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> index cbb6ad4..54b88bd 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
> @@ -56,6 +56,59 @@ static void exynos_dmabuf_detach(struct dma_buf *dmabuf,
> dma_buf_put(dmabuf);
> }
>
> +
Useless newline.
> +static struct sg_table *drm_dc_pages_to_sgt(struct page **pages,
> + unsigned long n_pages, size_t offset, size_t offset2, dma_addr_t daddr)
> +{
> + struct sg_table *sgt;
> + struct scatterlist *s;
> + int i, j, cur_page, chunks, ret;
> +
> + sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
> + if (!sgt)
> + return ERR_PTR(-ENOMEM);
> +
> + /* compute number of chunks */
> + chunks = 1;
> + for (i = 1; i < n_pages; ++i)
> + if (pages[i] != pages[i - 1] + 1)
> + ++chunks;
> +
> + ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
> + if (ret) {
> + kfree(sgt);
> + return ERR_PTR(-ENOMEM);
> + }
> +
> + /* merging chunks and putting them into the scatterlist */
> + cur_page = 0;
> + for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
> + size_t size = PAGE_SIZE;
> +
> + for (j = cur_page + 1; j < n_pages; ++j) {
> + if (pages[j] != pages[j - 1] + 1)
> + break;
> + size += PAGE_SIZE;
> + }
> +
> + /* cut offset if chunk starts at the first page */
> + if (cur_page == 0)
> + size -= offset;
> + /* cut offset2 if chunk ends at the last page */
> + if (j == n_pages)
> + size -= offset2;
> +
> + sg_set_page(s, pages[cur_page], size, offset);
> + s->dma_address = daddr;
> + daddr += size;
> + offset = 0;
> + cur_page = j;
> + }
> +
> + return sgt;
> +}
This function is almost same as Tomasz one at v4l2.
To Tomasz,
Can you make it common helper function at common place?
> +
> +
Useless newline too.
> static struct sg_table *exynos_map_dmabuf(struct dma_buf_attachment
> *attach,
> enum dma_data_direction direction)
> {
> @@ -64,6 +117,8 @@ static struct sg_table *exynos_map_dmabuf(struct
> dma_buf_attachment *attach,
> struct exynos_drm_gem_buf *buffer;
> struct sg_table *sgt;
> int ret;
> + int size, n_pages;
> + struct page **pages = NULL;
>
> DRM_DEBUG_KMS("%s\n", __FILE__);
>
> @@ -71,27 +126,37 @@ static struct sg_table *exynos_map_dmabuf(struct
> dma_buf_attachment *attach,
>
> buffer = exynos_gem_obj->buffer;
>
> - /* TODO. consider physically non-continuous memory with IOMMU. */
> + size = buffer->size;
> + n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>
> - sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
> - if (!sgt) {
> - DRM_DEBUG_KMS("failed to allocate sg table.\n");
> - return ERR_PTR(-ENOMEM);
> + pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
kcalloc?
Thank you,
Kyungmin Park
> + if (!pages) {
> + DRM_DEBUG_KMS("failed to alloc page table\n");
> + return NULL;
> }
>
> - ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
> + ret = dma_get_pages(attach->dev, buffer->kvaddr,
> + buffer->dma_addr, pages, n_pages);
> if (ret < 0) {
> - DRM_DEBUG_KMS("failed to allocate scatter list.\n");
> - kfree(sgt);
> - sgt = NULL;
> - return ERR_PTR(-ENOMEM);
> + DRM_DEBUG_KMS("failed to get buffer pages from DMA API\n");
> + return NULL;
> }
> + if (ret != n_pages) {
> + DRM_DEBUG_KMS("failed to get all pages from DMA API\n&qu
Re: [PATCH 1/4] [RFC] drm/exynos: DMABUF: Added support for exporting non-contig buffers
Hi,
I saw your description but please generate the patches against the
latest codes at next time.
On 4/14/12, Prathyush prathyus...@samsung.com wrote:
With this change, the exynos drm dmabuf module can export and
import dmabuf of gem objects with non-continuous memory.
The exynos_map_dmabuf function can create SGT of a non-contiguous
buffer by calling dma_get_pages to retrieve the allocated pages
and then maps the SGT to the caller's address space.
Signed-off-by: Prathyush K prathyus...@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_dmabuf.c | 98
+++-
1 files changed, 81 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index cbb6ad4..54b88bd 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -56,6 +56,59 @@ static void exynos_dmabuf_detach(struct dma_buf *dmabuf,
dma_buf_put(dmabuf);
}
+
Useless newline.
+static struct sg_table *drm_dc_pages_to_sgt(struct page **pages,
+ unsigned long n_pages, size_t offset, size_t offset2, dma_addr_t daddr)
+{
+ struct sg_table *sgt;
+ struct scatterlist *s;
+ int i, j, cur_page, chunks, ret;
+
+ sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
+ if (!sgt)
+ return ERR_PTR(-ENOMEM);
+
+ /* compute number of chunks */
+ chunks = 1;
+ for (i = 1; i n_pages; ++i)
+ if (pages[i] != pages[i - 1] + 1)
+ ++chunks;
+
+ ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ /* merging chunks and putting them into the scatterlist */
+ cur_page = 0;
+ for_each_sg(sgt-sgl, s, sgt-orig_nents, i) {
+ size_t size = PAGE_SIZE;
+
+ for (j = cur_page + 1; j n_pages; ++j) {
+ if (pages[j] != pages[j - 1] + 1)
+ break;
+ size += PAGE_SIZE;
+ }
+
+ /* cut offset if chunk starts at the first page */
+ if (cur_page == 0)
+ size -= offset;
+ /* cut offset2 if chunk ends at the last page */
+ if (j == n_pages)
+ size -= offset2;
+
+ sg_set_page(s, pages[cur_page], size, offset);
+ s-dma_address = daddr;
+ daddr += size;
+ offset = 0;
+ cur_page = j;
+ }
+
+ return sgt;
+}
This function is almost same as Tomasz one at v4l2.
To Tomasz,
Can you make it common helper function at common place?
+
+
Useless newline too.
static struct sg_table *exynos_map_dmabuf(struct dma_buf_attachment
*attach,
enum dma_data_direction direction)
{
@@ -64,6 +117,8 @@ static struct sg_table *exynos_map_dmabuf(struct
dma_buf_attachment *attach,
struct exynos_drm_gem_buf *buffer;
struct sg_table *sgt;
int ret;
+ int size, n_pages;
+ struct page **pages = NULL;
DRM_DEBUG_KMS(%s\n, __FILE__);
@@ -71,27 +126,37 @@ static struct sg_table *exynos_map_dmabuf(struct
dma_buf_attachment *attach,
buffer = exynos_gem_obj-buffer;
- /* TODO. consider physically non-continuous memory with IOMMU. */
+ size = buffer-size;
+ n_pages = PAGE_ALIGN(size) PAGE_SHIFT;
- sgt = kzalloc(sizeof(*sgt), GFP_KERNEL);
- if (!sgt) {
- DRM_DEBUG_KMS(failed to allocate sg table.\n);
- return ERR_PTR(-ENOMEM);
+ pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
kcalloc?
Thank you,
Kyungmin Park
+ if (!pages) {
+ DRM_DEBUG_KMS(failed to alloc page table\n);
+ return NULL;
}
- ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
+ ret = dma_get_pages(attach-dev, buffer-kvaddr,
+ buffer-dma_addr, pages, n_pages);
if (ret 0) {
- DRM_DEBUG_KMS(failed to allocate scatter list.\n);
- kfree(sgt);
- sgt = NULL;
- return ERR_PTR(-ENOMEM);
+ DRM_DEBUG_KMS(failed to get buffer pages from DMA API\n);
+ return NULL;
}
+ if (ret != n_pages) {
+ DRM_DEBUG_KMS(failed to get all pages from DMA API\n);
+ return NULL;
+ }
+
+ sgt = drm_dc_pages_to_sgt(pages, n_pages, 0, 0, buffer-dma_addr);
+ if (IS_ERR(sgt)) {
+ DRM_DEBUG_KMS(failed to prepare sg table\n);
+ return NULL;
+ }
+
+ sgt-nents = dma_map_sg(attach-dev, sgt-sgl,
+ sgt-orig_nents, DMA_BIDIRECTIONAL);
- sg_init_table(sgt-sgl, 1);
- sg_dma_len(sgt-sgl) = buffer-size;
- sg_set_page(sgt-sgl, pfn_to_page(PFN_DOWN(buffer-dma_addr)),
- buffer-size, 0);
- sg_dma_address(sgt-sgl
Test application for DMABUF sharing between V4L2 and DRM
Hi Tomasz,
How about to add this program to tools directory? tools/drm or tools/media?
Thank you,
Kyungmin Park
On Wed, Apr 11, 2012 at 1:13 AM, Tomasz Stanislawski
wrote:
> Hi Everyone,
> This email contains a test application showing DMABUF sharing
> between DRM/KMS display and capture node including VIVI.
> It shows a simple preview on LCD display. The similar application
> was posted in thread:
> http://thread.gmane.org/gmane.comp.video.dri.devel/65997
>
> This version makes use of single-plane API for V4L2 capture instead
> of multiplanar. This change allows VIVI driver to be tested as
> DMABUF importer.
>
> The program is written in C99 and it was tested using Exynos/DRM
> and VIVI capture.
>
> This application shows how buffer sharing between V4L2/DRM may look like.
> Please let me know if/where I use DRM/V4L2 incorrectly.
>
> The application was tested against 3.4-rc1 kernel with patches:
>
> - Integration of videobuf2 with dmabuf
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/46586
>
> - Integration of vb2-vmalloc and VIVI with dmabuf
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/46713
>
> - Support for DRM prime for Exynos by Inki Dae
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/exynos-drm-prime
>
> - patch fixing kmap/vmap support for EXYNOS-DRM prime.
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/3c483f24e418f342eac40dc5fb3991e058deb270
>
>
> The branch containing all mentioned patches (without platform code)
> rebased on 3.4-rc1 is available at link:
>
> http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/v3.4-rc1-v4l-drm-dmabuf-for-test
>
> Regards,
> Tomasz Stanislawski
>
> ---
>
>
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
> #include
>
> #include
> #include
> #include
>
> #define ERRSTR strerror(errno)
>
> #define BYE_ON(cond, ...) \
> do { \
> ? ? ? ?if (cond) { \
> ? ? ? ? ? ? ? ?int errsv = errno; \
> ? ? ? ? ? ? ? ?fprintf(stderr, "ERROR(%s:%d) : ", \
> ? ? ? ? ? ? ? ? ? ? ? ?__FILE__, __LINE__); \
> ? ? ? ? ? ? ? ?errno = errsv; \
> ? ? ? ? ? ? ? ?fprintf(stderr, ?__VA_ARGS__); \
> ? ? ? ? ? ? ? ?abort(); \
> ? ? ? ?} \
> } while(0)
>
> static inline int warn(const char *file, int line, const char *fmt, ...)
> {
> ? ? ? ?int errsv = errno;
> ? ? ? ?va_list va;
> ? ? ? ?va_start(va, fmt);
> ? ? ? ?fprintf(stderr, "WARN(%s:%d): ", file, line);
> ? ? ? ?vfprintf(stderr, fmt, va);
> ? ? ? ?va_end(va);
> ? ? ? ?errno = errsv;
> ? ? ? ?return 1;
> }
>
> #define WARN_ON(cond, ...) \
> ? ? ? ?((cond) ? warn(__FILE__, __LINE__, __VA_ARGS__) : 0)
>
> struct setup {
> ? ? ? ?char module[32];
> ? ? ? ?uint32_t conId;
> ? ? ? ?uint32_t crtId;
> ? ? ? ?char modestr[32];
> ? ? ? ?char video[32];
> ? ? ? ?unsigned int w, h;
> ? ? ? ?unsigned int use_wh : 1;
> ? ? ? ?unsigned int in_fourcc;
> ? ? ? ?unsigned int out_fourcc;
> ? ? ? ?unsigned int buffer_count;
> ? ? ? ?unsigned int use_crop : 1;
> ? ? ? ?unsigned int use_compose : 1;
> ? ? ? ?struct v4l2_rect crop;
> ? ? ? ?struct v4l2_rect compose;
> };
>
> struct buffer {
> ? ? ? ?unsigned int bo_handle;
> ? ? ? ?unsigned int fb_handle;
> ? ? ? ?int dbuf_fd;
> };
>
> struct stream {
> ? ? ? ?int v4lfd;
> ? ? ? ?int current_buffer;
> ? ? ? ?int buffer_count;
> ? ? ? ?struct buffer *buffer;
> } stream;
>
> static void usage(char *name)
> {
> ? ? ? ?fprintf(stderr, "usage: %s [-Moisth]\n", name);
> ? ? ? ?fprintf(stderr, "\t-M \tset DRM module\n");
> ? ? ? ?fprintf(stderr, "\t-o ::\tset a mode\n");
> ? ? ? ?fprintf(stderr, "\t-i \tset video node like
> /dev/video*\n");
> ? ? ? ?fprintf(stderr, "\t-S <width,height>\tset input resolution\n");
> ? ? ? ?fprintf(stderr, "\t-f \tset input format using 4cc\n");
> ? ? ? ?fprintf(stderr, "\t-F \tset output format using 4cc\n");
> ? ? ? ?fprintf(stderr, "\t-s <width,height>@<left,top>\tset crop area\n");
> ? ? ? ?fprintf(stderr, "\t-t <width,height>@<left,top>\tset compose area\n");
> ? ? ? ?fprintf(stderr, "\t-b buffer_count\tset number of buffers\n");
> ? ? ? ?fprintf(stderr, "\t-h\tshow this help\n");
> ? ? ? ?fprintf(stderr, "\n\tDefault is to dump all info.\n");
> }
>
> static inline int parse_rect(char *s, struct v4l2_rect *r)
> {
> ? ? ? ?return
Re: Test application for DMABUF sharing between V4L2 and DRM
Hi Tomasz,
How about to add this program to tools directory? tools/drm or tools/media?
Thank you,
Kyungmin Park
On Wed, Apr 11, 2012 at 1:13 AM, Tomasz Stanislawski
t.stanisl...@samsung.com wrote:
Hi Everyone,
This email contains a test application showing DMABUF sharing
between DRM/KMS display and capture node including VIVI.
It shows a simple preview on LCD display. The similar application
was posted in thread:
http://thread.gmane.org/gmane.comp.video.dri.devel/65997
This version makes use of single-plane API for V4L2 capture instead
of multiplanar. This change allows VIVI driver to be tested as
DMABUF importer.
The program is written in C99 and it was tested using Exynos/DRM
and VIVI capture.
This application shows how buffer sharing between V4L2/DRM may look like.
Please let me know if/where I use DRM/V4L2 incorrectly.
The application was tested against 3.4-rc1 kernel with patches:
- Integration of videobuf2 with dmabuf
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/46586
- Integration of vb2-vmalloc and VIVI with dmabuf
http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/46713
- Support for DRM prime for Exynos by Inki Dae
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/exynos-drm-prime
- patch fixing kmap/vmap support for EXYNOS-DRM prime.
http://git.infradead.org/users/kmpark/linux-2.6-samsung/commit/3c483f24e418f342eac40dc5fb3991e058deb270
The branch containing all mentioned patches (without platform code)
rebased on 3.4-rc1 is available at link:
http://git.infradead.org/users/kmpark/linux-2.6-samsung/shortlog/refs/heads/v3.4-rc1-v4l-drm-dmabuf-for-test
Regards,
Tomasz Stanislawski
---
#include errno.h
#include fcntl.h
#include linux/videodev2.h
#include math.h
#include poll.h
#include stdio.h
#include stdint.h
#include stdlib.h
#include string.h
#include sys/ioctl.h
#include sys/mman.h
#include sys/stat.h
#include sys/types.h
#include unistd.h
#include xf86drm.h
#include xf86drmMode.h
#include exynos_drm.h
#define ERRSTR strerror(errno)
#define BYE_ON(cond, ...) \
do { \
if (cond) { \
int errsv = errno; \
fprintf(stderr, ERROR(%s:%d) : , \
__FILE__, __LINE__); \
errno = errsv; \
fprintf(stderr, __VA_ARGS__); \
abort(); \
} \
} while(0)
static inline int warn(const char *file, int line, const char *fmt, ...)
{
int errsv = errno;
va_list va;
va_start(va, fmt);
fprintf(stderr, WARN(%s:%d): , file, line);
vfprintf(stderr, fmt, va);
va_end(va);
errno = errsv;
return 1;
}
#define WARN_ON(cond, ...) \
((cond) ? warn(__FILE__, __LINE__, __VA_ARGS__) : 0)
struct setup {
char module[32];
uint32_t conId;
uint32_t crtId;
char modestr[32];
char video[32];
unsigned int w, h;
unsigned int use_wh : 1;
unsigned int in_fourcc;
unsigned int out_fourcc;
unsigned int buffer_count;
unsigned int use_crop : 1;
unsigned int use_compose : 1;
struct v4l2_rect crop;
struct v4l2_rect compose;
};
struct buffer {
unsigned int bo_handle;
unsigned int fb_handle;
int dbuf_fd;
};
struct stream {
int v4lfd;
int current_buffer;
int buffer_count;
struct buffer *buffer;
} stream;
static void usage(char *name)
{
fprintf(stderr, usage: %s [-Moisth]\n, name);
fprintf(stderr, \t-M drm-module\tset DRM module\n);
fprintf(stderr, \t-o connector_id:crtc_id:mode\tset a mode\n);
fprintf(stderr, \t-i video-node\tset video node like
/dev/video*\n);
fprintf(stderr, \t-S width,height\tset input resolution\n);
fprintf(stderr, \t-f fourcc\tset input format using 4cc\n);
fprintf(stderr, \t-F fourcc\tset output format using 4cc\n);
fprintf(stderr, \t-s width,height@left,top\tset crop area\n);
fprintf(stderr, \t-t width,height@left,top\tset compose area\n);
fprintf(stderr, \t-b buffer_count\tset number of buffers\n);
fprintf(stderr, \t-h\tshow this help\n);
fprintf(stderr, \n\tDefault is to dump all info.\n);
}
static inline int parse_rect(char *s, struct v4l2_rect *r)
{
return sscanf(s, %d,%d@%d,%d, r-width, r-height,
r-top, r-left) != 4;
}
static int parse_args(int argc, char *argv[], struct setup *s)
{
if (argc = 1)
usage(argv[0]);
int c, ret;
memset(s, 0, sizeof(*s));
while ((c = getopt(argc, argv, M:o:i:S:f:F:s:t:b:h)) != -1) {
switch (c) {
case 'M':
strncpy(s-module, optarg, 31);
break;
case 'o':
ret = sscanf(optarg, %u:%u:%31s, s-conId,
s
[PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats
+ V4L2 develper
Sylwester, please give your opinions? How to handle it at v4l2 side.
Thank you,
Kyungmin Park
On Apr 6, 2012 12:44 AM, "Ville Syrj?l?" wrote:
> On Fri, Apr 06, 2012 at 03:05:36PM +0900, Inki Dae wrote:
> > Hi Ville,
> >
> > > -Original Message-
> > > From: Ville Syrj?l? [mailto:ville.syrjala at linux.intel.com]
> > > Sent: Friday, April 06, 2012 3:14 AM
> > > To: airlied at redhat.com
> > > Cc: inki.dae at samsung.com; kyungmin.park at samsung.com; dri-
> > > devel at lists.freedesktop.org; Seung-Woo Kim
> > > Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from
> kernel to
> > > add multi plane formats
> > >
> > > On Fri, Mar 30, 2012 at 01:12:58PM +0300, Ville Syrj?l? wrote:
> > > > On Fri, Mar 30, 2012 at 11:54:50AM +0900, Seung-Woo Kim wrote:
> > > > > Multi buffer plane pixel formats are added as like kernel header.
> > > > >
> > > > > Signed-off-by: Seung-Woo Kim
> > > > > ---
> > > > > include/drm/drm_fourcc.h |7 +++
> > > > > 1 files changed, 7 insertions(+), 0 deletions(-)
> > > > >
> > > > > diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> > > > > index 85facb0..7cfd95a 100644
> > > > > --- a/include/drm/drm_fourcc.h
> > > > > +++ b/include/drm/drm_fourcc.h
> > > > > @@ -107,6 +107,10 @@
> > > > > #define DRM_FORMAT_NV16fourcc_code('N', 'V', '1',
> '6') /*
> > > 2x1 subsampled Cr:Cb plane */
> > > > > #define DRM_FORMAT_NV61fourcc_code('N', 'V', '6',
> '1') /*
> > > 2x1 subsampled Cb:Cr plane */
> > > > >
> > > > > +/* 2 non contiguous plane YCbCr */
> > > > > +#define DRM_FORMAT_NV12M fourcc_code('N', 'M', '1', '2') /*
> 2x2
> > > subsampled Cr:Cb plane */
> > > >
> > > > NAK. DRM_FORMAT_NV12 handles this just fine.
> > >
> > > And I just realized that I was already too late with my NAK since this
> a
> > > libdrm patch. Apparently the kernel drm_fourcc.h changes were snuck in
> > > via some backdoor without review. Sigh.
> > >
> >
> > We had already requested review for it. for this you can refer to link
> > below:
> >
> >
> http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html
>
> I see. I couldn't find it in my work mailbox for some reason, and I
> don't remember having seen the patch before. I suppose I just missed it
> due to Christmas vacations, and was too blind to see it in my mailbox.
> Also google decicded to filter my search results too much, so I didn't
> spot it via the web archives either. I'm sorry for the false accusation.
>
> > > So they're now in Linus's tree. But looks like format_check() was never
> > > updated to accept them, so there's no way anyone could actually be
> using
> > > them. So Dave, can we still remove them from the kernel header?
> > >
> >
> > Yes, right. these formats aren't used for any SoCs except Exynos series
> yet
> > but just we are first. I think they should be added because anyone may
> use
> > them someday at least possible.
>
> Since DRM_FORMAT_NV12M is _identical_ to DRM_FORMAT_NV12, I see no point
> in adding it (similarly for YUV420M vs. YUV420).
>
> --
> Ville Syrj?l?
> syrjala at sci.fi
> http://www.sci.fi/~syrjala/
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL:
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120406/efd63853/attachment.htm>
Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from kernel to add multi plane formats
+ V4L2 develper
Sylwester, please give your opinions? How to handle it at v4l2 side.
Thank you,
Kyungmin Park
On Apr 6, 2012 12:44 AM, Ville Syrjälä syrj...@sci.fi wrote:
On Fri, Apr 06, 2012 at 03:05:36PM +0900, Inki Dae wrote:
Hi Ville,
-Original Message-
From: Ville Syrjälä [mailto:ville.syrj...@linux.intel.com]
Sent: Friday, April 06, 2012 3:14 AM
To: airl...@redhat.com
Cc: inki@samsung.com; kyungmin.p...@samsung.com; dri-
de...@lists.freedesktop.org; Seung-Woo Kim
Subject: Re: [PATCH libdrm] libdrm: update drm/drm_fourcc.h from
kernel to
add multi plane formats
On Fri, Mar 30, 2012 at 01:12:58PM +0300, Ville Syrjälä wrote:
On Fri, Mar 30, 2012 at 11:54:50AM +0900, Seung-Woo Kim wrote:
Multi buffer plane pixel formats are added as like kernel header.
Signed-off-by: Seung-Woo Kim sw0312@samsung.com
---
include/drm/drm_fourcc.h |7 +++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 85facb0..7cfd95a 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -107,6 +107,10 @@
#define DRM_FORMAT_NV16fourcc_code('N', 'V', '1',
'6') /*
2x1 subsampled Cr:Cb plane */
#define DRM_FORMAT_NV61fourcc_code('N', 'V', '6',
'1') /*
2x1 subsampled Cb:Cr plane */
+/* 2 non contiguous plane YCbCr */
+#define DRM_FORMAT_NV12M fourcc_code('N', 'M', '1', '2') /*
2x2
subsampled Cr:Cb plane */
NAK. DRM_FORMAT_NV12 handles this just fine.
And I just realized that I was already too late with my NAK since this
a
libdrm patch. Apparently the kernel drm_fourcc.h changes were snuck in
via some backdoor without review. Sigh.
We had already requested review for it. for this you can refer to link
below:
http://lists.freedesktop.org/archives/dri-devel/2011-December/017654.html
I see. I couldn't find it in my work mailbox for some reason, and I
don't remember having seen the patch before. I suppose I just missed it
due to Christmas vacations, and was too blind to see it in my mailbox.
Also google decicded to filter my search results too much, so I didn't
spot it via the web archives either. I'm sorry for the false accusation.
So they're now in Linus's tree. But looks like format_check() was never
updated to accept them, so there's no way anyone could actually be
using
them. So Dave, can we still remove them from the kernel header?
Yes, right. these formats aren't used for any SoCs except Exynos series
yet
but just we are first. I think they should be added because anyone may
use
them someday at least possible.
Since DRM_FORMAT_NV12M is _identical_ to DRM_FORMAT_NV12, I see no point
in adding it (similarly for YUV420M vs. YUV420).
--
Ville Syrjälä
syrj...@sci.fi
http://www.sci.fi/~syrjala/
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Linux 3.4-rc1
On Mon, Apr 2, 2012 at 12:34 AM, Rob Clark wrote: > On Sat, Mar 31, 2012 at 6:58 PM, Linus Torvalds > wrote: >> ?- drm dma-buf prime support. Dave Airlie sent me the pull request but >> didn't push very hard for it, it's in my "ok, I can still pull it for >> 3.4 if individual DRM driver people tell me that it will make their >> lives easier." So this is in limbo - I have nothing against it, but I >> won't pull unless I get a few people say "yes, please". > > yes, please :-) > > Note that the core drm dma-buf/prime support has already been reviewed > by a lot of folks, and tested with a few different drivers (exynos, > omapdrm, i915, nouveau, udl) with some driver support that could be > pushed for 3.5 cycle if the core support makes it in for 3.4 cycle. Yes, Please. It's used for exynos display and multimedia. Thank you, Kyungmin Park > > BR, > -R > > ___ > linaro-dev mailing list > linaro-dev at lists.linaro.org > http://lists.linaro.org/mailman/listinfo/linaro-dev
Re: Linux 3.4-rc1
On Mon, Apr 2, 2012 at 12:34 AM, Rob Clark rob.cl...@linaro.org wrote: On Sat, Mar 31, 2012 at 6:58 PM, Linus Torvalds torva...@linux-foundation.org wrote: - drm dma-buf prime support. Dave Airlie sent me the pull request but didn't push very hard for it, it's in my ok, I can still pull it for 3.4 if individual DRM driver people tell me that it will make their lives easier. So this is in limbo - I have nothing against it, but I won't pull unless I get a few people say yes, please. yes, please :-) Note that the core drm dma-buf/prime support has already been reviewed by a lot of folks, and tested with a few different drivers (exynos, omapdrm, i915, nouveau, udl) with some driver support that could be pushed for 3.5 cycle if the core support makes it in for 3.4 cycle. Yes, Please. It's used for exynos display and multimedia. Thank you, Kyungmin Park BR, -R ___ linaro-dev mailing list linaro-...@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[Linaro-mm-sig] [PATCH] dma-buf: add get_dma_buf()
Reviewed-by: Kyungmin Park
On 3/17/12, Rob Clark wrote:
> From: Rob Clark
>
> Works in a similar way to get_file(), and is needed in cases such as
> when the exporter needs to also keep a reference to the dmabuf (that
> is later released with a dma_buf_put()), and possibly other similar
> cases.
>
> Signed-off-by: Rob Clark
> ---
> Minor update on original to add a missing #include
>
> include/linux/dma-buf.h | 15 +++
> 1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> index 891457a..bc4203dc 100644
> --- a/include/linux/dma-buf.h
> +++ b/include/linux/dma-buf.h
> @@ -30,6 +30,7 @@
> #include
> #include
> #include
> +#include
>
> struct dma_buf;
> struct dma_buf_attachment;
> @@ -110,6 +111,20 @@ struct dma_buf_attachment {
> void *priv;
> };
>
> +/**
> + * get_dma_buf - convenience wrapper for get_file.
> + * @dmabuf: [in]pointer to dma_buf
> + *
> + * Increments the reference count on the dma-buf, needed in case of drivers
> + * that either need to create additional references to the dmabuf on the
> + * kernel side. For example, an exporter that needs to keep a dmabuf ptr
> + * so that subsequent exports don't create a new dmabuf.
> + */
> +static inline void get_dma_buf(struct dma_buf *dmabuf)
> +{
> + get_file(dmabuf->file);
> +}
> +
> #ifdef CONFIG_DMA_SHARED_BUFFER
> struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
> struct device *dev);
> --
> 1.7.5.4
>
>
> ___
> Linaro-mm-sig mailing list
> Linaro-mm-sig at lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
>
Re: [Linaro-mm-sig] [PATCH] dma-buf: add get_dma_buf()
Reviewed-by: Kyungmin Park kyungmin.p...@samsung.com
On 3/17/12, Rob Clark rob.cl...@linaro.org wrote:
From: Rob Clark r...@ti.com
Works in a similar way to get_file(), and is needed in cases such as
when the exporter needs to also keep a reference to the dmabuf (that
is later released with a dma_buf_put()), and possibly other similar
cases.
Signed-off-by: Rob Clark r...@ti.com
---
Minor update on original to add a missing #include
include/linux/dma-buf.h | 15 +++
1 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
index 891457a..bc4203dc 100644
--- a/include/linux/dma-buf.h
+++ b/include/linux/dma-buf.h
@@ -30,6 +30,7 @@
#include linux/scatterlist.h
#include linux/list.h
#include linux/dma-mapping.h
+#include linux/fs.h
struct dma_buf;
struct dma_buf_attachment;
@@ -110,6 +111,20 @@ struct dma_buf_attachment {
void *priv;
};
+/**
+ * get_dma_buf - convenience wrapper for get_file.
+ * @dmabuf: [in]pointer to dma_buf
+ *
+ * Increments the reference count on the dma-buf, needed in case of drivers
+ * that either need to create additional references to the dmabuf on the
+ * kernel side. For example, an exporter that needs to keep a dmabuf ptr
+ * so that subsequent exports don't create a new dmabuf.
+ */
+static inline void get_dma_buf(struct dma_buf *dmabuf)
+{
+ get_file(dmabuf-file);
+}
+
#ifdef CONFIG_DMA_SHARED_BUFFER
struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
struct device *dev);
--
1.7.5.4
___
Linaro-mm-sig mailing list
linaro-mm-...@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/14] update exynos drm driver.
Hi, Also you can find relevant patches at git http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/samsung-drm To Mr. Dae. Can you rebase the latest drm-next tree or mainline to merge easily? Thank you, Kyungmin Park On 11/12/11, Inki Dae wrote: > Hello, Dave. sorry but please, ignor previous patch sets posted > by Seung-Woo Kim. I am seding patch sets again. > > this patch sets are as the following. > - add kms poll to handle hdp event. > - fix converting between display mode and timing. > - fix connector and crtc callbacks to call proper display driver. > - fix vblank bug that manager->pipe could be -1 at vsync interrupt handler. > - add exynos_drm_gem_init() that is used commonly. > - change buffer structure to support IOMMU in the future. > - add comments and code clean. > > this patch is based on git repository below: > git://people.freedesktop.org/~airlied/linux.git > branch name: drm-fixes > commit-id: 8f3f1c9a22a6420e28c2d3eff59b832893bc8efc > > Inki Dae (7): > drm/exynos: added manager object to connector > drm/exynos: changed exynos_drm_display to exynos_drm_display_ops > drm/exynos: use gem create function generically > drm/exynos: removed unnecessary variable. > drm/exynos: changed buffer structure. > drm/exynos: fix vblank bug. > drm/exynos: include linux/module.h > > Joonyoung Shim (2): > drm/exynos: restored kernel_fb_list when reiniting fb_helper > drm/exynos: added crtc dpms for disable crtc > > Seung-Woo Kim (5): > drm/exynos: added kms poll for handling hpd event > drm/exynos: fixed connector flag with hpd and interlace scan for hdmi > drm/exynos: fixed converting between display mode and timing > drm/exynos: removed meaningless parameter from fbdev update > drm/exynos: checked for null pointer > > drivers/gpu/drm/exynos/exynos_drm_buf.c | 62 + > drivers/gpu/drm/exynos/exynos_drm_buf.h | 21 +- > drivers/gpu/drm/exynos/exynos_drm_connector.c | 78 +++-- > drivers/gpu/drm/exynos/exynos_drm_crtc.c | 76 ++-- > drivers/gpu/drm/exynos/exynos_drm_crtc.h | 25 +++ > drivers/gpu/drm/exynos/exynos_drm_drv.c |5 ++ > drivers/gpu/drm/exynos/exynos_drm_drv.h | 11 ++-- > drivers/gpu/drm/exynos/exynos_drm_encoder.c | 65 ++--- > drivers/gpu/drm/exynos/exynos_drm_encoder.h |1 + > drivers/gpu/drm/exynos/exynos_drm_fb.c| 66 ++--- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 44 +++ > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 62 - > drivers/gpu/drm/exynos/exynos_drm_gem.c | 94 > +++-- > drivers/gpu/drm/exynos/exynos_drm_gem.h | 28 ++-- > include/drm/exynos_drm.h |9 +-- > 15 files changed, 416 insertions(+), 231 deletions(-) > -- > 1.7.4.1 > > ___ > dri-devel mailing list > dri-devel at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel >
Re: [PATCH 00/14] update exynos drm driver.
Hi, Also you can find relevant patches at git http://git.infradead.org/users/kmpark/linux-samsung/shortlog/refs/heads/samsung-drm To Mr. Dae. Can you rebase the latest drm-next tree or mainline to merge easily? Thank you, Kyungmin Park On 11/12/11, Inki Dae inki@samsung.com wrote: Hello, Dave. sorry but please, ignor previous patch sets posted by Seung-Woo Kim. I am seding patch sets again. this patch sets are as the following. - add kms poll to handle hdp event. - fix converting between display mode and timing. - fix connector and crtc callbacks to call proper display driver. - fix vblank bug that manager-pipe could be -1 at vsync interrupt handler. - add exynos_drm_gem_init() that is used commonly. - change buffer structure to support IOMMU in the future. - add comments and code clean. this patch is based on git repository below: git://people.freedesktop.org/~airlied/linux.git branch name: drm-fixes commit-id: 8f3f1c9a22a6420e28c2d3eff59b832893bc8efc Inki Dae (7): drm/exynos: added manager object to connector drm/exynos: changed exynos_drm_display to exynos_drm_display_ops drm/exynos: use gem create function generically drm/exynos: removed unnecessary variable. drm/exynos: changed buffer structure. drm/exynos: fix vblank bug. drm/exynos: include linux/module.h Joonyoung Shim (2): drm/exynos: restored kernel_fb_list when reiniting fb_helper drm/exynos: added crtc dpms for disable crtc Seung-Woo Kim (5): drm/exynos: added kms poll for handling hpd event drm/exynos: fixed connector flag with hpd and interlace scan for hdmi drm/exynos: fixed converting between display mode and timing drm/exynos: removed meaningless parameter from fbdev update drm/exynos: checked for null pointer drivers/gpu/drm/exynos/exynos_drm_buf.c | 62 + drivers/gpu/drm/exynos/exynos_drm_buf.h | 21 +- drivers/gpu/drm/exynos/exynos_drm_connector.c | 78 +++-- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 76 ++-- drivers/gpu/drm/exynos/exynos_drm_crtc.h | 25 +++ drivers/gpu/drm/exynos/exynos_drm_drv.c |5 ++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 11 ++-- drivers/gpu/drm/exynos/exynos_drm_encoder.c | 65 ++--- drivers/gpu/drm/exynos/exynos_drm_encoder.h |1 + drivers/gpu/drm/exynos/exynos_drm_fb.c| 66 ++--- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 44 +++ drivers/gpu/drm/exynos/exynos_drm_fimd.c | 62 - drivers/gpu/drm/exynos/exynos_drm_gem.c | 94 +++-- drivers/gpu/drm/exynos/exynos_drm_gem.h | 28 ++-- include/drm/exynos_drm.h |9 +-- 15 files changed, 416 insertions(+), 231 deletions(-) -- 1.7.4.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] DRM: EXYNOS: Add EXYNOS DRM maintainer entry
From: Kyungmin Park <kyungmin.p...@samsung.com> As Exynos DRM is merged at mainline. Also update the maintainer entry. Signed-off-by: Kyungmin Park --- diff --git a/MAINTAINERS b/MAINTAINERS index 5d6941f..1deeac9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2327,6 +2327,13 @@ S: Supported F: drivers/gpu/drm/i915 F: include/drm/i915* +DRM DRIVERS FOR EXYNOS +M: Inki Dae +L: dri-devel at lists.freedesktop.org +S: Supported +F: drivers/gpu/drm/exynos +F: include/drm/exynos* + DSCC4 DRIVER M: Francois Romieu L: netdev at vger.kernel.org
[PATCH] DRM: EXYNOS: Add EXYNOS DRM maintainer entry
From: Kyungmin Park kyungmin.p...@samsung.com As Exynos DRM is merged at mainline. Also update the maintainer entry. Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- diff --git a/MAINTAINERS b/MAINTAINERS index 5d6941f..1deeac9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2327,6 +2327,13 @@ S: Supported F: drivers/gpu/drm/i915 F: include/drm/i915* +DRM DRIVERS FOR EXYNOS +M: Inki Dae inki@samsung.com +L: dri-devel@lists.freedesktop.org +S: Supported +F: drivers/gpu/drm/exynos +F: include/drm/exynos* + DSCC4 DRIVER M: Francois Romieu rom...@fr.zoreil.com L: net...@vger.kernel.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Proposal for a low-level Linux display framework
Hi Tomi, On Thu, Sep 15, 2011 at 9:07 PM, Tomi Valkeinen wrote: > Hi, > > I am the author of OMAP display driver, and while developing it I've > often felt that there's something missing in Linux's display area. I've > been planning to write a post about this for a few years already, but I > never got to it. So here goes at last! > > --- > > First I want to (try to) describe shortly what we have on OMAP, to give > a bit of a background for my point of view, and to have an example HW. > > The display subsystem (DSS) hardware on OMAP handles only showing pixels > on a display, so it doesn't contain anything that produces pixels like > 3D stuff or accelerated copying. All it does is fetch pixels from SDRAM, > possibly do some modifications for them (color format conversions etc), > and output them to a display. > > The hardware has multiple overlays, which are like hardware windows. > They fetch pixels from SDRAM, and output them in a certain area on the > display (possibly with scaling). Multiple overlays can be composited > into one output. > > So we may have something like this, when all overlays read pixels from > separate areas in the memory, and all overlays are on LCD display: > > ?.-. ? ? ? ? .--. ? ? ? ? ? .--. > ?| mem |>| ovl0 |-.>| LCD ?| > ?'-' ? ? ? ? '--' ? ? | ? ? '--' > ?.-. ? ? ? ? .--. ? ? | > ?| mem |>| ovl1 |-| > ?'-' ? ? ? ? '--' ? ? | > ?.-. ? ? ? ? .--. ? ? | ? ? .--. > ?| mem |>| ovl2 |-' ? ? | ?TV ?| > ?'-' ? ? ? ? '--' ? ? ? ? ? '--' > Same feature at samsung display subsystem. > The LCD display can be rather simple one, like a standard monitor or a > simple panel directly connected to parallel RGB output, or a more > complex one. A complex panel needs something else than just > turn-it-on-and-go. This may involve sending and receiving messages > between OMAP and the panel, but more generally, there's need to have > custom code that handles the particular panel. And the complex panel is > not necessarily a panel at all, it may be a buffer chip between OMAP and > the actual panel. > > The software side can be divided into three parts: the lower level > omapdss driver, the lower level panel drivers, and higher level drivers > like omapfb, v4l2 and omapdrm. Current omapdrm codes use the omapfb and omapdss codes even though omapdrm is located drivers/staging, some time later it should be drivers/gpu/gem/omap. but it still uses the drivers/video/omap2/dss codes. In case of samsung DRM, it has almost similar codes for lowlevel access from the drivers/video/s3c-fb.c for FIMD and drivers/media/video/s5p-tv for HDMI. > > The omapdss driver handles the OMAP DSS hardware, and offers a kernel > internal API which the higher level drivers use. The omapdss does not > know anything about fb or drm, it just offers core display services. > > The panel drivers handle particular panels/chips. The panel driver may > be very simple in case of a conventional display, basically doing pretty > much nothing, or bigger piece of code, handling communication with the > panel. > > The higher level drivers handle buffers and tell omapdss things like > where to find the pixels, what size the overlays should be, and use the > omapdss API to turn displays on/off, etc. > > --- > > There are two things that I'm proposing to improve the Linux display > support: > > First, there should be a bunch of common video structs and helpers that > are independent of any higher level framework. Things like video > timings, mode databases, and EDID seem to be implemented multiple times > in the kernel. But there shouldn't be anything in those things that > depend on any particular display framework, so they could be implemented > just once and all the frameworks could use them. > > Second, I think there could be use for a common low level display > framework. Currently the lower level code (display HW handling, etc.) > and higher level code (buffer management, policies, etc) seem to be > usually tied together, like the fb framework or the drm. Granted, the > frameworks do not force that, and for OMAP we indeed have omapfb and > omapdrm using the lower level omapdss. But I don't see that it's > anything OMAP specific as such. So I suggest the create the drivers/graphics for lowlevel codes and each framework, DRM, V4L2 and FB uses these lowlevel codes. Thank you, Kyungmin Park > > I think the lower level framework could have components something like > this (the naming is OMAP oriented, of course): > > overlay - a hardware "window", gets pixels from memory, possibly does > format conversions, scaling, etc. > > overlay compositor - com
[Linaro-mm-sig] [PATCH v4] DRM: add DRM Driver for Samsung SoC EXYNOS4210.
; This is to avoid the possibility that future drivers that need CMA will >>>>>> be vulnerable to DOS-attacks from ill-behaved DRI clients. >>>>>> >>>>>> >>>>> >>>>> Thomas, if any application has root authority for ill-purpose then isn't >>>>> >>>> >>>> it >>>> >>>>> >>>>> possible to be vulnerable to DOS-attacks? I think DRM_AUTH means root >>>>> authority. I know DRM Framework gives any root application DRM_AUTH >>>>> authority for compatibility. >>>>> >>>> >>>> DRM_AUTH just means that the client has authenticated w/ X11 (meaning >>>> that it has permission to connect to x server).. >>>> >>>> >>> >>> Yes, I understood so. but see drm_open_helper() of drm_fops.c file please. >>> in this function, you can see a line below. >>> /* for compatibility root is always authenticated */ >>> priv->authenticated = capable(CAP_SYS_ADMIN) >>> >>> I think the code above says that any application with root permission is >>> authenticated. >>> >>> >> >> Yes, that is true. A root client may be assumed to have AUTH permissions, >> but the inverse does not hold, meaning that an AUTH client may *not* be >> assumed to have root permissions. I think there is a ROOT_ONLY ioctl flag >> for that. >> >> The problem I'm seeing compared to other drivers is the following: >> >> Imagine for example that you have a disc driver that allocates temporary >> memory out of the same DMA pool as the DRM driver. >> >> Now you have a video player that is a DRM client. It contains a security >> flaw and is compromized by somebody trying to play a specially crafted video >> stream. The video player starts to allocate gem buffers until it receives an >> -ENOMEM. Then it stops allocating and does nothing. >> >> Now the system tries an important disc access (paging for example). This >> fails, because the video player has exhausted all DMA memory and the disc >> driver fails to allocate. >> >> The system is dead. >> >> The point is: >> >> If there is a chance that other drivers will use the same DMA/CMA pool as >> the DRM driver, DRM must leave enough DMA/CMA memory for those drivers to >> work. > > ah, ok, I get your point > > >> The difference compared to other drm drivers: >> >> There are other drm drivers that work the same way, with a static allocator. >> For example "via" and "sis". But those drivers completely claim the >> resources they are using. Nobody else is expected to use VRAM / AGP. >> >> In the Samsung case, it's not clear to me whether the DMA/CMA pool *can* be >> shared with other devices. >> If it is, IMHO you must implement an allocation limit in DRM, if not, the >> driver should probably be safe. > > It is possible to create a device private CMA pool.. ?although OTOH it > might be desirable to let some other drivers (like v4l2) use buffers > from the same pool.. That's the final goal. memory sharing among multimedia devices (v4l2), display(fimd, hdmi), and 3D (mali). currently mali drivers is tightly coupled with UMP and it's hard to use the CMA. it's need to solve this issue also. In case of multimedia drivers, it has fixed memory area as scenario and display also has similar restriction. I don't expect DRI request the more memory than CMA pool has. > > I'm not entirely sure what will happen w/ dma_alloc_coherent, etc, if > the global CMA pool is exhausted. The primary goal of CMA is guarantee the memory for multimedia. So if other device use the multimedia CMA pool, it should release the used memory. and it's hard to release the memory. it should has display CMA pool and don't share the other multimedia devices CMA pool. CMA can share the same CMA pool and specify the device usage which CMA pool used. > > Marek? ?I guess you know what would happen? He will comeback when 21 Sep. Thank you, Kyungmin Park
Re: [Linaro-mm-sig] [PATCH v4] DRM: add DRM Driver for Samsung SoC EXYNOS4210.
this issue also. In case of multimedia drivers, it has fixed memory area as scenario and display also has similar restriction. I don't expect DRI request the more memory than CMA pool has. I'm not entirely sure what will happen w/ dma_alloc_coherent, etc, if the global CMA pool is exhausted. The primary goal of CMA is guarantee the memory for multimedia. So if other device use the multimedia CMA pool, it should release the used memory. and it's hard to release the memory. it should has display CMA pool and don't share the other multimedia devices CMA pool. CMA can share the same CMA pool and specify the device usage which CMA pool used. Marek? I guess you know what would happen? He will comeback when 21 Sep. Thank you, Kyungmin Park ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Proposal for a low-level Linux display framework
Hi Tomi, On Thu, Sep 15, 2011 at 9:07 PM, Tomi Valkeinen tomi.valkei...@ti.com wrote: Hi, I am the author of OMAP display driver, and while developing it I've often felt that there's something missing in Linux's display area. I've been planning to write a post about this for a few years already, but I never got to it. So here goes at last! --- First I want to (try to) describe shortly what we have on OMAP, to give a bit of a background for my point of view, and to have an example HW. The display subsystem (DSS) hardware on OMAP handles only showing pixels on a display, so it doesn't contain anything that produces pixels like 3D stuff or accelerated copying. All it does is fetch pixels from SDRAM, possibly do some modifications for them (color format conversions etc), and output them to a display. The hardware has multiple overlays, which are like hardware windows. They fetch pixels from SDRAM, and output them in a certain area on the display (possibly with scaling). Multiple overlays can be composited into one output. So we may have something like this, when all overlays read pixels from separate areas in the memory, and all overlays are on LCD display: .-. .--. .--. | mem || ovl0 |-.| LCD | '-' '--' | '--' .-. .--. | | mem || ovl1 |-| '-' '--' | .-. .--. | .--. | mem || ovl2 |-' | TV | '-' '--' '--' Same feature at samsung display subsystem. The LCD display can be rather simple one, like a standard monitor or a simple panel directly connected to parallel RGB output, or a more complex one. A complex panel needs something else than just turn-it-on-and-go. This may involve sending and receiving messages between OMAP and the panel, but more generally, there's need to have custom code that handles the particular panel. And the complex panel is not necessarily a panel at all, it may be a buffer chip between OMAP and the actual panel. The software side can be divided into three parts: the lower level omapdss driver, the lower level panel drivers, and higher level drivers like omapfb, v4l2 and omapdrm. Current omapdrm codes use the omapfb and omapdss codes even though omapdrm is located drivers/staging, some time later it should be drivers/gpu/gem/omap. but it still uses the drivers/video/omap2/dss codes. In case of samsung DRM, it has almost similar codes for lowlevel access from the drivers/video/s3c-fb.c for FIMD and drivers/media/video/s5p-tv for HDMI. The omapdss driver handles the OMAP DSS hardware, and offers a kernel internal API which the higher level drivers use. The omapdss does not know anything about fb or drm, it just offers core display services. The panel drivers handle particular panels/chips. The panel driver may be very simple in case of a conventional display, basically doing pretty much nothing, or bigger piece of code, handling communication with the panel. The higher level drivers handle buffers and tell omapdss things like where to find the pixels, what size the overlays should be, and use the omapdss API to turn displays on/off, etc. --- There are two things that I'm proposing to improve the Linux display support: First, there should be a bunch of common video structs and helpers that are independent of any higher level framework. Things like video timings, mode databases, and EDID seem to be implemented multiple times in the kernel. But there shouldn't be anything in those things that depend on any particular display framework, so they could be implemented just once and all the frameworks could use them. Second, I think there could be use for a common low level display framework. Currently the lower level code (display HW handling, etc.) and higher level code (buffer management, policies, etc) seem to be usually tied together, like the fb framework or the drm. Granted, the frameworks do not force that, and for OMAP we indeed have omapfb and omapdrm using the lower level omapdss. But I don't see that it's anything OMAP specific as such. So I suggest the create the drivers/graphics for lowlevel codes and each framework, DRM, V4L2 and FB uses these lowlevel codes. Thank you, Kyungmin Park I think the lower level framework could have components something like this (the naming is OMAP oriented, of course): overlay - a hardware window, gets pixels from memory, possibly does format conversions, scaling, etc. overlay compositor - composes multiple overlays into one output, possibly doing things like translucency. output - gets the pixels from overlay compositor, and sends them out according to particular video timings when using conventional video interface, or via any other mean when using non-conventional video buses like DSI command mode. display - handles an external display
[RFC][PATCH v3] DRM: add DRM Driver for Samsung SoC EXYNOS4210.
On Thu, Sep 1, 2011 at 10:06 PM, Inki Dae wrote:
> Hello Rob.
> Below is my comments. thank you.
>
>> -Original Message-
>> From: Rob Clark [mailto:robdclark at gmail.com]
>> Sent: Thursday, September 01, 2011 1:02 AM
>> To: Inki Dae
>> Cc: airlied at linux.ie; dri-devel at lists.freedesktop.org;
>> sw0312.kim at samsung.com; linux-kernel at vger.kernel.org;
>> kyungmin.park at samsung.com; linux-arm-kernel at lists.infradead.org
>> Subject: Re: [RFC][PATCH v3] DRM: add DRM Driver for Samsung SoC
>> EXYNOS4210.
>>
>> On Wed, Aug 31, 2011 at 1:51 AM, Inki Dae wrote:
>> > Hello, Rob.
>> > Below is my answers and questions. and could you please include me as CC
>> > when you post your driver?
>>
>> sure thing
>>
>>
>> >> > +static int samsung_drm_connector_get_modes(struct drm_connector
>> >> *connector)
>> >> > +{
>> >> > + ? ? ? struct samsung_drm_connector *samsung_connector =
>> >> > + ? ? ? ? ? ? ? to_samsung_connector(connector);
>> >> > + ? ? ? struct samsung_drm_display *display =
>> >> > + ? ? ? ? ? ? ? samsung_drm_get_manager(samsung_connector->encoder)-
>> >> >display;
>> >> > + ? ? ? unsigned int count;
>> >> > +
>> >> > + ? ? ? DRM_DEBUG_KMS("%s\n", __FILE__);
>> >> > +
>> >> > + ? ? ? if (display && display->get_edid) {
>> >> > + ? ? ? ? ? ? ? void *edid = kzalloc(MAX_EDID, GFP_KERNEL);
>> >> > + ? ? ? ? ? ? ? if (!edid) {
>> >> > + ? ? ? ? ? ? ? ? ? ? ? DRM_ERROR("failed to allocate edid\n");
>> >> > + ? ? ? ? ? ? ? ? ? ? ? return 0;
>> >> > + ? ? ? ? ? ? ? }
>> >> > +
>> >> > + ? ? ? ? ? ? ? display->get_edid(connector, edid, MAX_EDID);
>> >> > +
>> >> > + ? ? ? ? ? ? ? drm_mode_connector_update_edid_property(connector,
>> > edid);
>> >> > + ? ? ? ? ? ? ? count = drm_add_edid_modes(connector, edid);
>> >> > +
>> >> > + ? ? ? ? ? ? ? kfree(connector->display_info.raw_edid);
>> >> > + ? ? ? ? ? ? ? connector->display_info.raw_edid = edid;
>> >> > + ? ? ? } else {
>> >> > + ? ? ? ? ? ? ? struct drm_display_mode *mode =
>> > drm_mode_create(connector-
>> >> >dev);
>> >> > + ? ? ? ? ? ? ? struct fb_videomode *timing;
>> >> > +
>> >> > + ? ? ? ? ? ? ? if (display && display->get_timing)
>> >> > + ? ? ? ? ? ? ? ? ? ? ? timing = display->get_timing();
>> >>
>> >> also seems a bit weird to not do: display->get_timings(display)..
>> >>
>> >> maybe you don't have the case of multiple instances of the same
>> >> display time.. but maybe someday you need that. ?(Maybe this is just
>> >> an artifact of how the API of your current lower layer driver is.. but
>> >> maybe now is a good time to think about those APIs)
>> >>
>> >
>> > display is static object set by hardware specific driver such as display
>> > controller or hdmi. each hardware specific driver has their own display
>> > object statically. You can refer to the definition in
> samsung_drm_fimd.c.
>> > and I understood a encoder and a connector is 1:1 relationship, when any
>> > hardware specific driver is probed, they would be created with a manager
>> > that includes their own display object as pointer. Could you please give
>> me
>> > more comments why display object should be considered for multiple
>> > instances? I am afraid there is any part I don't care.
>> >
>> > thank you.
>>
>> Just thinking hypothetically.. what if some future device had two hdmi
>> controllers. ?Then you'd want two instances of the same display
>> object.
>>
>> Although it seems this API is just internal to the DRM driver (which I
>> had not realized earlier), so it should be pretty easy to change later
>> if needed.
>>
>
> You are right. I guess we should consider multiple instances to display
> object because the Exynos hardware has two display controllers and also each
> display controller needs one display panel with sharing same driver. thank
> you for your pointing. :)
Two HDMI and two FIMD is different. even though exynos4 supports the
two display controller. it has only one HDMI port and now there's no
device to use it. Of course if new device uses it. it will support it.
I mena currently make
Re: [RFC][PATCH v3] DRM: add DRM Driver for Samsung SoC EXYNOS4210.
On Thu, Sep 1, 2011 at 10:06 PM, Inki Dae inki@samsung.com wrote:
Hello Rob.
Below is my comments. thank you.
-Original Message-
From: Rob Clark [mailto:robdcl...@gmail.com]
Sent: Thursday, September 01, 2011 1:02 AM
To: Inki Dae
Cc: airl...@linux.ie; dri-devel@lists.freedesktop.org;
sw0312@samsung.com; linux-ker...@vger.kernel.org;
kyungmin.p...@samsung.com; linux-arm-ker...@lists.infradead.org
Subject: Re: [RFC][PATCH v3] DRM: add DRM Driver for Samsung SoC
EXYNOS4210.
On Wed, Aug 31, 2011 at 1:51 AM, Inki Dae inki@samsung.com wrote:
Hello, Rob.
Below is my answers and questions. and could you please include me as CC
when you post your driver?
sure thing
+static int samsung_drm_connector_get_modes(struct drm_connector
*connector)
+{
+ struct samsung_drm_connector *samsung_connector =
+ to_samsung_connector(connector);
+ struct samsung_drm_display *display =
+ samsung_drm_get_manager(samsung_connector-encoder)-
display;
+ unsigned int count;
+
+ DRM_DEBUG_KMS(%s\n, __FILE__);
+
+ if (display display-get_edid) {
+ void *edid = kzalloc(MAX_EDID, GFP_KERNEL);
+ if (!edid) {
+ DRM_ERROR(failed to allocate edid\n);
+ return 0;
+ }
+
+ display-get_edid(connector, edid, MAX_EDID);
+
+ drm_mode_connector_update_edid_property(connector,
edid);
+ count = drm_add_edid_modes(connector, edid);
+
+ kfree(connector-display_info.raw_edid);
+ connector-display_info.raw_edid = edid;
+ } else {
+ struct drm_display_mode *mode =
drm_mode_create(connector-
dev);
+ struct fb_videomode *timing;
+
+ if (display display-get_timing)
+ timing = display-get_timing();
also seems a bit weird to not do: display-get_timings(display)..
maybe you don't have the case of multiple instances of the same
display time.. but maybe someday you need that. (Maybe this is just
an artifact of how the API of your current lower layer driver is.. but
maybe now is a good time to think about those APIs)
display is static object set by hardware specific driver such as display
controller or hdmi. each hardware specific driver has their own display
object statically. You can refer to the definition in
samsung_drm_fimd.c.
and I understood a encoder and a connector is 1:1 relationship, when any
hardware specific driver is probed, they would be created with a manager
that includes their own display object as pointer. Could you please give
me
more comments why display object should be considered for multiple
instances? I am afraid there is any part I don't care.
thank you.
Just thinking hypothetically.. what if some future device had two hdmi
controllers. Then you'd want two instances of the same display
object.
Although it seems this API is just internal to the DRM driver (which I
had not realized earlier), so it should be pretty easy to change later
if needed.
You are right. I guess we should consider multiple instances to display
object because the Exynos hardware has two display controllers and also each
display controller needs one display panel with sharing same driver. thank
you for your pointing. :)
Two HDMI and two FIMD is different. even though exynos4 supports the
two display controller. it has only one HDMI port and now there's no
device to use it. Of course if new device uses it. it will support it.
I mena currently make it simple and make it TODO list.
Thank you,
Kyungmin Park
+static void samsung_drm_connector_destroy(struct drm_connector
*connector)
+{
+ struct samsung_drm_connector *samsung_connector =
+ to_samsung_connector(connector);
+
+ DRM_DEBUG_KMS(%s\n, __FILE__);
+
+ drm_sysfs_connector_remove(connector);
+ drm_connector_cleanup(connector);
+ connector-dev-mode_config.num_connector--;
I wonder if num_connector-- should be in drm_connector_cleanup()..
I missed this in OMAP driver, but it seems that none of the other drm
drivers are directly decrementing this.. and it would seem more
symmetric for it to be in drm_connector_cleanup(). But would be
For this, it was already commented by Joonyoun Shim.
And it seems (which I hadn't noticed earlier) already merged :-)
So I think this (and similar bit in encoder destructor) can be removed
+static void samsung_drm_fb_destroy(struct drm_framebuffer *fb)
+{
+ struct samsung_drm_fb *samsung_fb = to_samsung_fb(fb);
+ int ret;
+
+ DRM_DEBUG_KMS(%s\n, __FILE__);
+
+ drm_framebuffer_cleanup(fb);
+
+ if (samsung_fb-is_default) {
+ ret
Review request to DRM Driver for Samsung SoC Exynos4210.
CC Eric (freescale), Rob (TI) who working the DRM with other ARM SoCs. As Dave mentioned, Please review Samsung DRM codes. Thank you, Kyungmin Park -Original Message- From: Dave Airlie [mailto:airl...@gmail.com] Sent: Monday, August 29, 2011 11:27 PM To: Inki Dae Cc: airlied at linux.ie; kyungmin.park at samsung.com; dri-devel at lists.freedesktop.org Subject: Re: Review request to DRM Driver for Samsung SoC Exynos4210. > I had sent DRM Driver for Samsung SoC Exynos4210 over the three times. > > But I didn't received any comments from you. I was sort of hoping someone else would take a look at these ARM drivers before me, it might be worth getting some inter-company review between the vendors submitting drm components as I'm guessing its going to be a lot of the same thing. But I've done a quick review and some things that stick out. 1. include/drm/samsung_drm.h should only contain public information for use on the userspace ioctl interface, it shouldn't contain any kernel private data structures, they should be in drivers/gpu/drm/samsung/samsung_drv.h or something very similiar. 2. I'm not sure why samsung_drm_fn_encoder exists, it looks like from the crtc mode set functions you call the encoder mode set functions, is it not possible to use the helper drm_crtc_helper_set_mode so that the crtc mode set is called then the encoder ones without having the explicit calls? If not please either document it or point at the explaination I missed. 3. Not terribly urgent but is samsung the best name for this? what happens if you bring out another chip, I also think there is a lot of samsung_drm_ in function names that seems not that useful. dropping _drm_ might help. 4 ioctls: drm_samsung_gem_map_off needs explicit padding before the 64-bit, drm_samsung_gem_mmap needs explicit padding before the 64-bit,, though I'm not sure you need these ioctls all now that the dumb interface is upstream, what is usr_addr in the gem create ioctl for? this seems wrong, it also looks too short for 64-bit addresses, but passing in userspace addr to kernel is generally not a great plan. 5. you probably don't need master_create/set ops. Dave.
RE: Review request to DRM Driver for Samsung SoC Exynos4210.
CC Eric (freescale), Rob (TI) who working the DRM with other ARM SoCs. As Dave mentioned, Please review Samsung DRM codes. Thank you, Kyungmin Park -Original Message- From: Dave Airlie [mailto:airl...@gmail.com] Sent: Monday, August 29, 2011 11:27 PM To: Inki Dae Cc: airl...@linux.ie; kyungmin.p...@samsung.com; dri-devel@lists.freedesktop.org Subject: Re: Review request to DRM Driver for Samsung SoC Exynos4210. I had sent DRM Driver for Samsung SoC Exynos4210 over the three times. But I didn't received any comments from you. I was sort of hoping someone else would take a look at these ARM drivers before me, it might be worth getting some inter-company review between the vendors submitting drm components as I'm guessing its going to be a lot of the same thing. But I've done a quick review and some things that stick out. 1. include/drm/samsung_drm.h should only contain public information for use on the userspace ioctl interface, it shouldn't contain any kernel private data structures, they should be in drivers/gpu/drm/samsung/samsung_drv.h or something very similiar. 2. I'm not sure why samsung_drm_fn_encoder exists, it looks like from the crtc mode set functions you call the encoder mode set functions, is it not possible to use the helper drm_crtc_helper_set_mode so that the crtc mode set is called then the encoder ones without having the explicit calls? If not please either document it or point at the explaination I missed. 3. Not terribly urgent but is samsung the best name for this? what happens if you bring out another chip, I also think there is a lot of samsung_drm_ in function names that seems not that useful. dropping _drm_ might help. 4 ioctls: drm_samsung_gem_map_off needs explicit padding before the 64-bit, drm_samsung_gem_mmap needs explicit padding before the 64-bit,, though I'm not sure you need these ioctls all now that the dumb interface is upstream, what is usr_addr in the gem create ioctl for? this seems wrong, it also looks too short for 64-bit addresses, but passing in userspace addr to kernel is generally not a great plan. 5. you probably don't need master_create/set ops. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
