Re: [PATCH v2 08/13] exynos: fimg2d: add g2d_set_direction

2015-11-26 Thread Hyungwon Hwang
Dear all,

Yes. I can pick it on behalf of Tobias, if there is no one who can do
that.

BRs,
Hyungwon Hwang

On Thu, 26 Nov 2015 16:48:10 +
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> On 26 November 2015 at 16:41, Tobias Jakobi <liquid.a...@gmx.net>
> wrote:
> > Hello Emil,
> >
> > my main system which I also used for development was stolen on
> > Tuesday, so I won't be working on this series anytime soon. If
> > anyone wants to pick it up, please go ahead.
> >
> Sad to hear that x2. You've been doing some good work, despite that we
> weren't always quick to review/pick it. Hope you had some insurance
> and you manage to sort things out, relatively stress free.
> 
> Inki, Hyungwon, other Exynos devs,
> 
> Will you guys be able to pick any of this work ? Most of it can go in
> with the small suggestion in this patch addressed.
> 
> Thanks
> Emil
> 

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


Re: [PATCH v2 00/13] drm/exynos: async G2D and g2d_move()

2015-11-26 Thread Hyungwon Hwang
Dear All,

On Thu, 26 Nov 2015 16:35:29 +
Emil Velikov <emil.l.veli...@gmail.com> wrote:

> Hi Tobias,
> 
> On 22 November 2015 at 18:48, Tobias Jakobi
> <tjak...@math.uni-bielefeld.de> wrote:
> > Hello,
> >
> > this series mostly touches G2D code. It introduces the following:
> >
> > (1) drmHandleEvent2() is added to enable processing of
> > vendor-specific events. This will be used to expose asynchronous
> > operation of the G2D. The necessary kernel infrastructure is
> > already there since a lot of kernel versions. [This touches libdrm
> > core code!]
> >
> Considering the shortage of input on this can you please respin the
> series without it (and related work mentioned below). This way we can
> pick merge the remaining work now and discuss (ping) about the rest.

I am confused a little here. Did you mean that adding drmHandleEvent2()
is not necessary, and the related code which uses drmHandleEvent2() must
be re-implemented?

> 
> > (2) The necessary infrastructure to handle G2D events. This includes
> > adding g2d_config_event() and g2d_exec2() to the public API.
> > A test application is provided to ensure that everything works
> > as expected.
> >
> With above in mind the g2d event will need to be split out, although
> g2d_exec2() should be ok (although of limited use), imho.
> 
> > (3) A small performance test application which can be used to
> > measure the speed of solid color clear operations. Interesting for
> > benchmarking and plotting colorful graphs (e.g. through
> > Mathematica).
> >
> > (4) g2d_move() which works similar to g2d_copy() but like the C
> > memmove() properly handles overlapping buffer copies.
> > Again a test application is present to check that this
> > indeed does what it should.
> >
> > (5) Various small changes. A framebuffer colorformat fix for the
> > general G2D test application. Moving the currently unused
> > g2d_reset() to the public API.
> I am more of a "add API when it's needed" kind of person, although I
> cannot see anything serious misuse that can arise from g2d_reset().
> 
> > Adding a counterpart to
> > exynos_bo_map() to unmap buffers again.
> >
> The exynos bo map compatibility was broken a few times already so I'm
> wondering if we really want this one. I guess that with the lack of
> any (outside of tizen) user space things cannot go that wrong :-P
> 

Yes. I agree that adding them at this time is not needed and it would
be OK to add them, when the userspace using them comes.

BRs,
Hyungwon Hwang

> Thanks
> Emil
> 

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


Re: [PATCH v2 00/13] drm/exynos: async G2D and g2d_move()

2015-11-22 Thread Hyungwon Hwang
Hello Tobias,

I reviewed this whole patchset, and it looks good to me. Also I tested
it on my odroid u3, and all works fine. Thanks you for your effort.

Tested-by: Hyungwon Hwang <human.hw...@samsung.com>
Reviewed-by: Hyungwon Hwang <human.hw...@samsung.com>

BRs,
Hyungwon Hwang

On Sun, 22 Nov 2015 19:48:30 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello,
> 
> this series mostly touches G2D code. It introduces the following:
> 
> (1) drmHandleEvent2() is added to enable processing of vendor-specific
> events. This will be used to expose asynchronous operation of the
> G2D. The necessary kernel infrastructure is already there since
> a lot of kernel versions. [This touches libdrm core code!]
> 
> (2) The necessary infrastructure to handle G2D events. This includes
> adding g2d_config_event() and g2d_exec2() to the public API.
> A test application is provided to ensure that everything works
> as expected.
> 
> (3) A small performance test application which can be used to measure
> the speed of solid color clear operations. Interesting for
> benchmarking and plotting colorful graphs (e.g. through
> Mathematica).
> 
> (4) g2d_move() which works similar to g2d_copy() but like the C
> memmove() properly handles overlapping buffer copies.
> Again a test application is present to check that this
> indeed does what it should.
> 
> (5) Various small changes. A framebuffer colorformat fix for the
> general G2D test application. Moving the currently unused
> g2d_reset() to the public API. Adding a counterpart to
> exynos_bo_map() to unmap buffers again.
> 
> (6) Last but not least a small bump of the Exynos version number.
> 
> Please review and let me know what I should change/improve.
> 
> 
> With best wishes,
> Tobias
> 
> P.S.: Most patches were submitted already some time ago but never
> made it upstream. So if something looks familiar, don't worry! ;)
> 
> Changes since v1:
> - Added wording changes suggested by Hyungwon Hwang.
> - Added binaries for new test applications to .gitignore.
> - Collected r-b and t-b tags.
> 
> Tobias Jakobi (13):
>   drm: Implement drmHandleEvent2()
>   exynos: Introduce exynos_handle_event()
>   tests/exynos: add fimg2d performance analysis
>   exynos/fimg2d: add g2d_config_event
>   exynos: fimg2d: add g2d_exec2
>   tests/exynos: add fimg2d event test
>   tests/exynos: use XRGB for framebuffer
>   exynos: fimg2d: add g2d_set_direction
>   exynos/fimg2d: add g2d_move
>   tests/exynos: add test for g2d_move
>   exynos/fimg2d: add exynos_bo_unmap()
>   exynos/fimg2d: add g2d_reset() to public API
>   exynos: bump version number
> 
>  .gitignore |   2 +
>  exynos/exynos-symbol-check |   5 +
>  exynos/exynos_drm.c|  48 ++
>  exynos/exynos_drm.h|  12 ++
>  exynos/exynos_drmif.h  |  27 +++
>  exynos/exynos_fimg2d.c | 165 +--
>  exynos/exynos_fimg2d.h |  49 ++
>  exynos/libdrm_exynos.pc.in |   2 +-
>  tests/exynos/Makefile.am   |  26 ++-
>  tests/exynos/exynos_fimg2d_event.c | 326
> 
> tests/exynos/exynos_fimg2d_perf.c  | 327
> +
> tests/exynos/exynos_fimg2d_test.c  | 134 ++-
> xf86drm.h  |  21 +++
> xf86drmMode.c  |  10 +- 14 files changed, 1138
> insertions(+), 16 deletions(-) create mode 100644
> tests/exynos/exynos_fimg2d_event.c create mode 100644
> tests/exynos/exynos_fimg2d_perf.c
> 

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


Re: [PATCH v4] tests/exynos: add fimg2d performance analysis

2015-11-10 Thread Hyungwon Hwang
On Tue, 10 Nov 2015 14:23:51 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello Hyungwon,
> 
> 
> Hyungwon Hwang wrote:
> > Hello Tobias,
> > 
> > On Mon, 09 Nov 2015 10:47:13 +0100
> > Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:
> > 
> >> Hello Hyungwon,
> >>
> >>
> >> Hyungwon Hwang wrote:
> >>> Hello,
> >>>
> >>> I think this patch should update .gitignore, not for adding the
> >>> built binary to untracked file list.
> >> Thanks!
> >>
> >>
> >>> Also, I want to make clear about the purpose of this test program.
> >>> What do you want to get after this test? This program runs G2D
> >>> with randomly chosen number of pixel and shows the elapsed time
> >>> to do that. I run it on my board. But I could not find any
> >>> meaning of the test. If you just want to know the execution time
> >>> of solid fill, what about get the width and height from user and
> >>> run the same tests iteratively for more accurate result? Or at
> >>> least, increasing number of pixels?
> >> The test is to measure the dependency between amount of pixels the
> >> G2D has to process and the amount of time for the G2D to process
> >> such pixels.
> >>
> >> It's exactly what a performance test should do, measure the time it
> >> takes for a certain workload to complete.
> >>
> >> In particular the test wants to answer the question if the
> >> dependency stated above is of linear type.
> >>
> >> Of course it's not, since we have setup time, so at least it
> >> should be affine linear. But even that is not true, since you see
> >> subtle 'branching' when doing high density plots (that's why I
> >> added export of the data to Mathematica).
> >>
> >>
> >> What you ask for (user input) is in fact already implemented. The
> >> user can specify the buffer width and height, which in turn limits
> >> the size of the rectangle that is solid filled.
> >>
> >> If you want smaller rectangles filled, decrease buffer width and
> >> height, if you want bigger ones filled, increase.
> >>
> >>
> >> The second purpose is to stress test the G2D, as already indicated
> >> in the commit description. The G2D can be overclocked quite a lot
> >> under certain conditions. With increase MIF/INT voltages I can run
> >> it with 400MHz instead of the 200MHz defaults. The application can
> >> now be used to check stability. E.g. if voltages are too low the
> >> system can quickly lock-up.
> >>
> >> In particular one could also check how processing time depends on
> >> the clock rate of the G2D. One interesting question here is how
> >> memory bandwidth limits us.
> >>
> >>
> >>
> >> With best wishes,
> >> Tobias
> > 
> > Yes. I agree with the broad view. Please see the below, I run the
> > test 2 times in a row.
> > 
> > root@localhost:~# ./exynos_fimg2d_perf  -i 10 -w 1024 -h 1024   
> > exynos/fimg2d: G2D version (4.1).
> > starting simple G2D performance test
> > buffer width = 1024, buffer height = 1024, iterations = 10
> > num_pixels = 136000, usecs = 236000
> > num_pixels = 8492, usecs = 47083
> > num_pixels = 100688, usecs = 200042
> > num_pixels = 141312, usecs = 216667
> > num_pixels = 39962, usecs = 92708
> > num_pixels = 95046, usecs = 156542
> > num_pixels = 2562, usecs = 34666
> > num_pixels = 176485, usecs = 326916
> > num_pixels = 17760, usecs = 56625
> > num_pixels = 1625, usecs = 31833
> > starting multi G2D performance test (batch size = 3)
> > buffer width = 1024, buffer height = 1024, iterations = 10
> > num_pixels = 245180, usecs = 385083
> > num_pixels = 276320, usecs = 398625
> > num_pixels = 196807, usecs = 35
> > num_pixels = 305540, usecs = 420458
> > num_pixels = 65978, usecs = 120250
> > num_pixels = 265028, usecs = 379417
> > num_pixels = 139079, usecs = 213667
> > num_pixels = 24970, usecs = 67625
> > num_pixels = 46808, usecs = 114125
> > num_pixels = 100804, usecs = 179750
> > root@localhost:~# ./exynos_fimg2d_perf  -i 10 -w 1024 -h 1024 
> > exynos/fimg2d: G2D version (4.1).
> > starting simple G2D performance test
> > buffer width = 1024, buffer height = 1024, iterations = 10
> > num_pixels = 18676, usecs = 95541
> > num_pixels = 117056, usecs = 218875
> > num_pixels = 8078

Re: [PATCH 09/13] exynos/fimg2d: add g2d_move

2015-11-10 Thread Hyungwon Hwang
Hello Tobias,

On Tue, 10 Nov 2015 14:24:11 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello Hyungwon,
> 
> 
> Hyungwon Hwang wrote:
> > Hello Tobias,
> > 
> > On Mon, 09 Nov 2015 10:47:02 +0100
> > Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:
> > 
> >> Hello Hyungwon,
> >>
> >>
> >> Hyungwon Hwang wrote:
> >>> Hello Tobias,
> >>>
> >>> I was in vacation last week, so I could run your code today. I
> >>> found that what g2d_move() does is actually copying not moving,
> >>> because the operation does not clear the previous area.
> >> I choose g2d_move() because we also have memcpy() and memmove() in
> >> libc. Like in libc 'moving' memory doesn't actually move it, like
> >> you would move a real object, since it's undefined what 'empty'
> >> memory should be.
> >>
> >> The same applies here. It's not defined what 'empty' areas of the
> >> buffer should be.
> >>
> >>
> >>> Would it be possible to
> >>> generalize g2d_copy() works better, so it could works well in case
> >>> of the src buffer and dst buffer being same.
> >> I think this would break g2d_copy() backward compatibility.
> >>
> >> I also want the user to explicitly request this. The user should
> >> make sure what requirements he has for the buffers in question.
> >> Are the buffers disjoint or not?
> >>
> >>
> >>> If it is possible, I think it
> >>> would be better way to do that. If it is not, at least chaning the
> >>> function name is needed. I tested it on my Odroid U3 board.
> >> I don't have a strong opinion on the naming. Any recommendations?
> >>
> >> I still think the naming is fine though, since it mirrors libc's
> >> naming. And the user is supposed to read the documentation anyway.
> > 
> >>
> >>
> >>
> >> With best wishes,
> >> Tobias
> > 
> > In that manner following glibc, I agree that the naming is
> > reasonable.
> well, that was just my way of thinking. But I guess most people have
> experience using the libc, so the naming should look at least
> 'familiar'.
> 
> 
> 
> > I commented like that previously, because at the first time when I
> > run the test, I think that the result seems like a bug. The test
> > program said that it was a move test, but the operation seemed
> > copying.
> Ok, so just that I understand this correctly. Your issue is with the
> commit the description of the test or with the commit description of
> the patch that introduces g2d_move()?
> 
> Because I don't see what you point out in the test commit description:
> 
> "
> tests/exynos: add test for g2d_move
> 
> To check if g2d_move() works properly we create a small checkerboard
> pattern in the center of the screen and then shift this pattern
> around with g2d_move(). The pattern should be properly preserved
> by the operation.
> "
> 
> I intentionally avoid to write "...move this pattern around...", so
> instead I choose "shift".
> 
> I'm not a native speaker, so I'm clueless how to formulate this in a
> more clear way.

I'm also not a native speaker, so maybe I could not figure out the
subtle difference between move and shift. I just thought that 'shift'
was just the synonym of 'move'.

> 
> 
> > It
> > would be just OK if it is well documented or printed when runs the
> > test that the test does not do anything about the previous area
> > intentionally.
> I could add solid fills of the 'empty' areas after each move()
> operation. Would that be more in line what you think the test should
> do?

No. Because g2d_move() is to g2d_copy() what memmove() is to memcpy(),
the current implementation seems enough.

What about change 'move' to 'copy' in the function description?

* g2d_move - copy content inside single buffer.
* Similar to 'memmove' this copies a rectangular region
* of the provided buffer to another location (the source
* and destination region potentially overlapping)

Best regards,
Hyungwon Hwang

> 
> 
> With best wishes,
> Tobias
> 
> 
> 
> 
> > 
> > 
> > BRs,
> > Hyungwon Hwang
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-samsung-soc" in the body of a message to
> majord...@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

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


Re: [PATCH v4] tests/exynos: add fimg2d performance analysis

2015-11-09 Thread Hyungwon Hwang
Hello Tobias,

On Mon, 09 Nov 2015 10:47:13 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello Hyungwon,
> 
> 
> Hyungwon Hwang wrote:
> > Hello,
> > 
> > I think this patch should update .gitignore, not for adding the
> > built binary to untracked file list.
> Thanks!
> 
> 
> > Also, I want to make clear about the purpose of this test program.
> > What do you want to get after this test? This program runs G2D with
> > randomly chosen number of pixel and shows the elapsed time to do
> > that. I run it on my board. But I could not find any meaning of the
> > test. If you just want to know the execution time of solid fill,
> > what about get the width and height from user and run the same tests
> > iteratively for more accurate result? Or at least, increasing
> > number of pixels?
> The test is to measure the dependency between amount of pixels the G2D
> has to process and the amount of time for the G2D to process such
> pixels.
> 
> It's exactly what a performance test should do, measure the time it
> takes for a certain workload to complete.
> 
> In particular the test wants to answer the question if the dependency
> stated above is of linear type.
> 
> Of course it's not, since we have setup time, so at least it should be
> affine linear. But even that is not true, since you see subtle
> 'branching' when doing high density plots (that's why I added export
> of the data to Mathematica).
> 
> 
> What you ask for (user input) is in fact already implemented. The user
> can specify the buffer width and height, which in turn limits the size
> of the rectangle that is solid filled.
> 
> If you want smaller rectangles filled, decrease buffer width and
> height, if you want bigger ones filled, increase.
> 
> 
> The second purpose is to stress test the G2D, as already indicated in
> the commit description. The G2D can be overclocked quite a lot under
> certain conditions. With increase MIF/INT voltages I can run it with
> 400MHz instead of the 200MHz defaults. The application can now be used
> to check stability. E.g. if voltages are too low the system can
> quickly lock-up.
> 
> In particular one could also check how processing time depends on the
> clock rate of the G2D. One interesting question here is how memory
> bandwidth limits us.
> 
> 
> 
> With best wishes,
> Tobias

Yes. I agree with the broad view. Please see the below, I run the test
2 times in a row.

root@localhost:~# ./exynos_fimg2d_perf  -i 10 -w 1024 -h 1024   
exynos/fimg2d: G2D version (4.1).
starting simple G2D performance test
buffer width = 1024, buffer height = 1024, iterations = 10
num_pixels = 136000, usecs = 236000
num_pixels = 8492, usecs = 47083
num_pixels = 100688, usecs = 200042
num_pixels = 141312, usecs = 216667
num_pixels = 39962, usecs = 92708
num_pixels = 95046, usecs = 156542
num_pixels = 2562, usecs = 34666
num_pixels = 176485, usecs = 326916
num_pixels = 17760, usecs = 56625
num_pixels = 1625, usecs = 31833
starting multi G2D performance test (batch size = 3)
buffer width = 1024, buffer height = 1024, iterations = 10
num_pixels = 245180, usecs = 385083
num_pixels = 276320, usecs = 398625
num_pixels = 196807, usecs = 35
num_pixels = 305540, usecs = 420458
num_pixels = 65978, usecs = 120250
num_pixels = 265028, usecs = 379417
num_pixels = 139079, usecs = 213667
num_pixels = 24970, usecs = 67625
num_pixels = 46808, usecs = 114125
num_pixels = 100804, usecs = 179750
root@localhost:~# ./exynos_fimg2d_perf  -i 10 -w 1024 -h 1024 
exynos/fimg2d: G2D version (4.1).
starting simple G2D performance test
buffer width = 1024, buffer height = 1024, iterations = 10
num_pixels = 18676, usecs = 95541
num_pixels = 117056, usecs = 218875
num_pixels = 80784, usecs = 137209
num_pixels = 427, usecs = 33209
num_pixels = 238044, usecs = 403041
num_pixels = 4392, usecs = 37709
num_pixels = 19880, usecs = 59750
num_pixels = 3666, usecs = 36542
num_pixels = 4630, usecs = 36166
num_pixels = 70834, usecs = 125917
starting multi G2D performance test (batch size = 3)
buffer width = 1024, buffer height = 1024, iterations = 10
num_pixels = 216516, usecs = 347042
num_pixels = 242863, usecs = 422417
num_pixels = 28176, usecs = 72292
num_pixels = 110713, usecs = 179167
num_pixels = 292266, usecs = 431750
num_pixels = 274127, usecs = 392833
num_pixels = 291659, usecs = 415875
num_pixels = 140202, usecs = 218833
num_pixels = 122400, usecs = 193084
num_pixels = 168647, usecs = 251375

As you said, I can adjust the buffer width and height. But because the
program choose the number of pixel to process randomly, I can't compare
the result after I modified something (clock, or something else like
you mentioned). Also I can figure out the tendency between the number
of pixels and the processing time after I draw the graph. But it i

Re: [PATCH 09/13] exynos/fimg2d: add g2d_move

2015-11-09 Thread Hyungwon Hwang
Hello Tobias,

On Mon, 09 Nov 2015 10:47:02 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello Hyungwon,
> 
> 
> Hyungwon Hwang wrote:
> > Hello Tobias,
> > 
> > I was in vacation last week, so I could run your code today. I found
> > that what g2d_move() does is actually copying not moving, because
> > the operation does not clear the previous area.
> I choose g2d_move() because we also have memcpy() and memmove() in
> libc. Like in libc 'moving' memory doesn't actually move it, like you
> would move a real object, since it's undefined what 'empty' memory
> should be.
> 
> The same applies here. It's not defined what 'empty' areas of the
> buffer should be.
> 
> 
> > Would it be possible to
> > generalize g2d_copy() works better, so it could works well in case
> > of the src buffer and dst buffer being same.
> I think this would break g2d_copy() backward compatibility.
> 
> I also want the user to explicitly request this. The user should make
> sure what requirements he has for the buffers in question. Are the
> buffers disjoint or not?
> 
> 
> > If it is possible, I think it
> > would be better way to do that. If it is not, at least chaning the
> > function name is needed. I tested it on my Odroid U3 board.
> I don't have a strong opinion on the naming. Any recommendations?
> 
> I still think the naming is fine though, since it mirrors libc's
> naming. And the user is supposed to read the documentation anyway.

> 
> 
> 
> With best wishes,
> Tobias

In that manner following glibc, I agree that the naming is reasonable.
I commented like that previously, because at the first time when I run
the test, I think that the result seems like a bug. The test program
said that it was a move test, but the operation seemed copying. It
would be just OK if it is well documented or printed when runs the test
that the test does not do anything about the previous area
intentionally.


BRs,
Hyungwon Hwang

> 
> > 
> > Best regards,
> > Hyungwon Hwang
> > 
> > On Tue, 22 Sep 2015 17:54:58 +0200
> > Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:
> > 
> >> We already have g2d_copy() which implements G2D copy
> >> operations from one buffer to another. However we can't
> >> do a overlapping copy operation in one buffer.
> >>
> >> Add g2d_move() which acts like the standard memmove()
> >> and properly handles overlapping copies.
> >>
> >> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> >> ---
> >>  exynos/exynos_fimg2d.c | 94
> >> ++
> >> exynos/exynos_fimg2d.h |  3 ++ 2 files changed, 97 insertions(+)
> >>
> >> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> >> index 4d5419c..8703629 100644
> >> --- a/exynos/exynos_fimg2d.c
> >> +++ b/exynos/exynos_fimg2d.c
> >> @@ -540,6 +540,100 @@ g2d_copy(struct g2d_context *ctx, struct
> >> g2d_image *src, }
> >>  
> >>  /**
> >> + * g2d_move - move content inside single buffer.
> >> + *Similar to 'memmove' this moves a rectangular region
> >> + *of the provided buffer to another location (the source
> >> + *and destination region potentially overlapping).
> >> + *
> >> + * @ctx: a pointer to g2d_context structure.
> >> + * @img: a pointer to g2d_image structure providing
> >> + *buffer information.
> >> + * @src_x: x position of source rectangle.
> >> + * @src_y: y position of source rectangle.
> >> + * @dst_x: x position of destination rectangle.
> >> + * @dst_y: y position of destination rectangle.
> >> + * @w: width of rectangle to move.
> >> + * @h: height of rectangle to move.
> >> + */
> >> +int
> >> +g2d_move(struct g2d_context *ctx, struct g2d_image *img,
> >> +  unsigned int src_x, unsigned int src_y,
> >> +  unsigned int dst_x, unsigned dst_y, unsigned int
> >> w,
> >> +  unsigned int h)
> >> +{
> >> +  union g2d_rop4_val rop4;
> >> +  union g2d_point_val pt;
> >> +  union g2d_direction_val dir;
> >> +  unsigned int src_w, src_h, dst_w, dst_h;
> >> +
> >> +  src_w = w;
> >> +  src_h = h;
> >> +  if (src_x + img->width > w)
> >> +  src_w = img->width - src_x;
> >> +  if (src_y + img->height > h)
> >> +  src_h = img->height - src_y;
> >> +
> >> +  dst_w = w;
> >> +  dst_h

Re: [PATCH v4] tests/exynos: add fimg2d performance analysis

2015-11-08 Thread Hyungwon Hwang
Hello,

I think this patch should update .gitignore, not for adding the built
binary to untracked file list.

Also, I want to make clear about the purpose of this test program. What
do you want to get after this test? This program runs G2D with
randomly chosen number of pixel and shows the elapsed time to do
that. I run it on my board. But I could not find any meaning of the
test. If you just want to know the execution time of solid fill, what
about get the width and height from user and run the same tests
iteratively for more accurate result? Or at least, increasing number of
pixels?


Best regards,
Hyungwon Hwang


On Mon, 02 Nov 2015 10:52:09 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Currently only fast solid color clear performance is measured.
> A large buffer is allocated and solid color clear operations
> are executed on it with randomly chosen properties (position
> and size of the region, clear color). Execution time is
> measured and output together with the amount of pixels
> processed.
> 
> The 'simple' variant only executes one G2D command buffer at
> a time, while the 'multi' variant executes multiple ones. This
> can be used to measure setup/exec overhead.
> 
> The test also serves a stability check. If clocks/voltages are
> too high or low respectively, the test quickly reveals this.
> 
> v2: Add GPLv2 header, argument handling and documentation.
> Tool is only installed when requested.
> v3: Free images array in fimg2d_perf_multi() as pointed out
> by Hyungwon Hwang.
> v4: Include header for error numbers (fixes build).
> 
> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> ---
>  tests/exynos/Makefile.am  |  19 ++-
>  tests/exynos/exynos_fimg2d_perf.c | 327
> ++ 2 files changed, 344
> insertions(+), 2 deletions(-) create mode 100644
> tests/exynos/exynos_fimg2d_perf.c
> 
> diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am
> index b21d016..e82d199 100644
> --- a/tests/exynos/Makefile.am
> +++ b/tests/exynos/Makefile.am
> @@ -5,16 +5,31 @@ AM_CFLAGS = \
>   -I $(top_srcdir)/exynos \
>   -I $(top_srcdir)
>  
> +bin_PROGRAMS =
> +noinst_PROGRAMS =
> +
>  if HAVE_LIBKMS
>  if HAVE_INSTALL_TESTS
> -bin_PROGRAMS = \
> +bin_PROGRAMS += \
>   exynos_fimg2d_test
>  else
> -noinst_PROGRAMS = \
> +noinst_PROGRAMS += \
>   exynos_fimg2d_test
>  endif
>  endif
>  
> +if HAVE_INSTALL_TESTS
> +bin_PROGRAMS += \
> + exynos_fimg2d_perf
> +else
> +noinst_PROGRAMS += \
> + exynos_fimg2d_perf
> +endif
> +
> +exynos_fimg2d_perf_LDADD = \
> + $(top_builddir)/libdrm.la \
> + $(top_builddir)/exynos/libdrm_exynos.la
> +
>  exynos_fimg2d_test_LDADD = \
>   $(top_builddir)/libdrm.la \
>   $(top_builddir)/libkms/libkms.la \
> diff --git a/tests/exynos/exynos_fimg2d_perf.c
> b/tests/exynos/exynos_fimg2d_perf.c new file mode 100644
> index 000..1699bba
> --- /dev/null
> +++ b/tests/exynos/exynos_fimg2d_perf.c
> @@ -0,0 +1,327 @@
> +/*
> + * Copyright (C) 2015 - Tobias Jakobi
> + *
> + * This is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> + * by the Free Software Foundation, either version 2 of the License,
> + * or (at your option) any later version.
> + *
> + * It is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + * You should have received a copy of the GNU General Public License
> + * along with it. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "exynos_drm.h"
> +#include "exynos_drmif.h"
> +#include "exynos_fimg2d.h"
> +
> +static int output_mathematica = 0;
> +
> +static int fimg2d_perf_simple(struct exynos_bo *bo, struct
> g2d_context *ctx,
> + unsigned buf_width, unsigned buf_height,
> unsigned iterations) +{
> + struct timespec tspec = { 0 };
> + struct g2d_image img = { 0 };
> +
> + unsigned long long g2d_time;
> + unsigned i;
> + int ret = 0;
> +
> + img.width = buf_width;
> + img.height = buf_height;
> + img.stride = buf_width * 4;
> + img.color_mode = G2D_COLOR_FMT_ARGB | G2D_ORDER_AXRGB;
> + img.buf_type = G2D_IMGBUF_GEM;
> + img.bo[0] = bo->handle;
> +
> + srand(time(NULL));
> +
> + printf("starti

Re: [PATCH 06/13] tests/exynos: add fimg2d event test

2015-11-01 Thread Hyungwon Hwang
On Fri, 30 Oct 2015 12:16:47 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello Hyungwon,
> 
> first of all thanks for reviewing the series!
> 
> 
> 
> Hyungwon Hwang wrote:
> > On Tue, 22 Sep 2015 17:54:55 +0200
> > Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:
> > 
> >> This tests async processing of G2D jobs. A separate thread is
> >> spawned to monitor the DRM fd for events and check whether a G2D
> >> job was completed.
> >>
> >> v2: Add GPLv2 header, argument handling and documentation.
> >> Test is only installed when requested.
> >> v3: Allocate G2D jobs with calloc which fixes 'busy' being
> >> potentially uninitialized. Also enable timeout for poll()
> >> in the monitor thread. This fixes pthread_join() not working
> >> because of poll() not returning.
> >>
> >> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> >> ---
> >>  tests/exynos/Makefile.am   |  11 +-
> >>  tests/exynos/exynos_fimg2d_event.c | 326
> >> + 2 files changed, 335
> >> insertions(+), 2 deletions(-) create mode 100644
> >> tests/exynos/exynos_fimg2d_event.c
> >>
> >> diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am
> >> index e82d199..357d6b8 100644
> >> --- a/tests/exynos/Makefile.am
> >> +++ b/tests/exynos/Makefile.am
> >> @@ -20,16 +20,23 @@ endif
> >>  
> >>  if HAVE_INSTALL_TESTS
> >>  bin_PROGRAMS += \
> >> -  exynos_fimg2d_perf
> >> +  exynos_fimg2d_perf \
> >> +  exynos_fimg2d_event
> >>  else
> >>  noinst_PROGRAMS += \
> >> -  exynos_fimg2d_perf
> >> +  exynos_fimg2d_perf \
> >> +  exynos_fimg2d_event
> >>  endif
> >>  
> >>  exynos_fimg2d_perf_LDADD = \
> >>$(top_builddir)/libdrm.la \
> >>$(top_builddir)/exynos/libdrm_exynos.la
> >>  
> >> +exynos_fimg2d_event_LDADD = \
> >> +  $(top_builddir)/libdrm.la \
> >> +  $(top_builddir)/exynos/libdrm_exynos.la \
> >> +  -lpthread
> >> +
> >>  exynos_fimg2d_test_LDADD = \
> >>$(top_builddir)/libdrm.la \
> >>$(top_builddir)/libkms/libkms.la \
> >> diff --git a/tests/exynos/exynos_fimg2d_event.c
> >> b/tests/exynos/exynos_fimg2d_event.c new file mode 100644
> >> index 000..c03dcff
> >> --- /dev/null
> >> +++ b/tests/exynos/exynos_fimg2d_event.c
> >> @@ -0,0 +1,326 @@
> >> +/*
> >> + * Copyright (C) 2015 - Tobias Jakobi
> >> + *
> >> + * This is free software: you can redistribute it and/or modify
> >> + * it under the terms of the GNU General Public License as
> >> published
> >> + * by the Free Software Foundation, either version 2 of the
> >> License,
> >> + * or (at your option) any later version.
> >> + *
> >> + * It is distributed in the hope that it will be useful, but
> >> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> >> + * GNU General Public License for more details.
> >> + * You should have received a copy of the GNU General Public
> >> License
> >> + * along with it. If not, see <http://www.gnu.org/licenses/>.
> >> + */
> >> +
> >> +#include 
> >> +#include 
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#include 
> >> +
> >> +#include 
> >> +
> >> +#include "exynos_drm.h"
> >> +#include "exynos_drmif.h"
> >> +#include "exynos_fimg2d.h"
> >> +
> >> +struct g2d_job {
> >> +  unsigned int id;
> >> +  unsigned int busy;
> >> +};
> >> +
> >> +struct exynos_evhandler {
> >> +  struct pollfd fds;
> >> +  struct exynos_event_context evctx;
> >> +};
> >> +
> >> +struct threaddata {
> >> +  unsigned int stop;
> >> +  struct exynos_device *dev;
> >> +  struct exynos_evhandler evhandler;
> >> +};
> >> +
> >> +static void g2d_event_handler(int fd, unsigned int cmdlist_no,
> >> unsigned int tv_sec,
> >> +  unsigned
> >> int tv_usec, void *user_data) +{
> >> +  struct g2d_job *job = user_data;
>

Re: [PATCH 07/13] tests/exynos: use XRGB8888 for framebuffer

2015-11-01 Thread Hyungwon Hwang
On Fri, 30 Oct 2015 12:17:02 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Hello Hyungwon,
> 
> 
> Hyungwon Hwang wrote:
> > On Tue, 22 Sep 2015 17:54:56 +0200
> > Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:
> > 
> >> This matches the G2D color mode that is used in the entire code.
> >> The previous (incorrect) RGBA would only work since the
> >> Exynos mixer did its configuration based on the bpp, and not
> >> based on the actual pixelformat.
> >>
> >> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> >> ---
> >>  tests/exynos/exynos_fimg2d_test.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/tests/exynos/exynos_fimg2d_test.c
> >> b/tests/exynos/exynos_fimg2d_test.c index 8794dac..dfb00a0 100644
> >> --- a/tests/exynos/exynos_fimg2d_test.c
> >> +++ b/tests/exynos/exynos_fimg2d_test.c
> >> @@ -675,7 +675,7 @@ int main(int argc, char **argv)
> >>offsets[0] = 0;
> >>  
> >>ret = drmModeAddFB2(dev->fd, screen_width, screen_height,
> >> -  DRM_FORMAT_RGBA, handles,
> >> +  DRM_FORMAT_XRGB, handles,
> >>pitches, offsets, _id, 0);
> > 
> > Reviewed-by: Hyungwon Hwang <human.hw...@samsung.com>
> > 
> > Nice catch. It's right, if there was no previous setting for source
> > image color mode. But I think it could be the source image color
> > mode was set by another application before when this test runs. So
> > I think the code which sets the source image color mode must be
> > added.
> I think you misunderstand something here. First of all settings from
> anither application using DRM don't carry over.
> 
> The point for this change is that all used G2D image structures have
> the color_mode field set to G2D_COLOR_FMT_ARGB | G2D_ORDER_AXRGB.
> So the G2D is operating on ARGB pixel data.
> 
> However the framebuffer is set to DRM_FORMAT_RGBA, which obviously
> is wrong since this is not the type of data we put into the
> framebuffer.

Oh. That's right. I misunderstanded the code. This patch is absolutely
right.

Best regards,
Hyungwon Hwang

> 
> 
> With best wishes,
> Tobias
> 
> 
> > 
> > Best regards,
> > Hyungwon Hwang
> > 
> > 
> >>if (ret < 0)
> >>goto err_destroy_buffer;
> > 
> 

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


Re: [PATCH 08/13] exynos: fimg2d: add g2d_set_direction

2015-11-01 Thread Hyungwon Hwang
On Fri, 30 Oct 2015 18:14:23 +0100
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> Tobias Jakobi wrote:
> > Hello Hyungwon,
> > 
> > 
> > Hyungwon Hwang wrote:
> >> On Tue, 22 Sep 2015 17:54:57 +0200
> >> Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:
> >>
> >>> This allows setting the two direction registers, which specify how
> >>> the engine blits pixels. This can be used for overlapping blits,
> >>> which happen e.g. when 'moving' a rectangular region inside a
> >>> fixed buffer.
> >>
> >> Code itself looks good. But as I know, direction registers are
> >> related with flip, not moving. Isn't it? I am not that much
> >> familiar with G2D. Please let me know if I am wrong.
> > that's correct, you can use the direction registers to implement
> > flipping operations.
> > 
> > However what they really do is to change the operation direction of
> > the G2D engine. So you can manage how the engine traverses through
> > pixel rows and column (either in forward or in backward direction).
> > 
> > Normally this doesn't matter, like it doesn't matter if you memcpy()
> > from one buffer to another when there is no intersection of the
> > buffers. However if the two buffers overlap then you have to be
> > careful with the direction in which you traverse the buffer.
> > 
> > Now if you set the x-direction of the source G2D image to forward,
> > but the x-direction of the destination to backward, you get your
> > mentioned x-flipping.
> > 
> > The main purpose for g2d_move() is to be used e.g. in acceleration
> > code for the armsoc Xorg DDX. A common operation there is not move
> > pixel regions around in the same buffer.
> Sorry, this should read "...there is to move pixel regions...".

OK. I understood what you meant.

Reviewed-by: Hyungwon Hwang <human.hw...@samsung.com>


Best regards,
Hyungwon Hwang

> 
> 
> - Tobias
> 
> > 
> > 
> > With best wishes,
> > Tobias
> > 
> > 
> > P.S.: I think the Exynos user manual mentions flipping as one
> > example on how to use these registers. But like I said above, it's
> > just one way to use it.
> > 
> > 
> > 
> > 
> >>
> >> Best regards,
> >> Hyungwon Hwang
> >>
> >>>
> >>> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> >>> ---
> >>>  exynos/exynos_fimg2d.c | 13 +
> >>>  exynos/exynos_fimg2d.h | 38
> >>> ++ 2 files changed, 51
> >>> insertions(+)
> >>>
> >>> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> >>> index e997d4b..4d5419c 100644
> >>> --- a/exynos/exynos_fimg2d.c
> >>> +++ b/exynos/exynos_fimg2d.c
> >>> @@ -242,6 +242,19 @@ static void g2d_add_base_addr(struct
> >>> g2d_context *ctx, struct g2d_image *img, }
> >>>  
> >>>  /*
> >>> + * g2d_set_direction - setup direction register (useful for
> >>> overlapping blits).
> >>> + *
> >>> + * @ctx: a pointer to g2d_context structure.
> >>> + * @dir: a pointer to the g2d_direction_val structure.
> >>> + */
> >>> +static void g2d_set_direction(struct g2d_context *ctx,
> >>> + const union g2d_direction_val *dir)
> >>> +{
> >>> + g2d_add_cmd(ctx, SRC_MASK_DIRECT_REG, dir->val[0]);
> >>> + g2d_add_cmd(ctx, DST_PAT_DIRECT_REG, dir->val[1]);
> >>> +}
> >>> +
> >>> +/*
> >>>   * g2d_reset - reset fimg2d hardware.
> >>>   *
> >>>   * @ctx: a pointer to g2d_context structure.
> >>> diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h
> >>> index c6b58ac..9eee7c0 100644
> >>> --- a/exynos/exynos_fimg2d.h
> >>> +++ b/exynos/exynos_fimg2d.h
> >>> @@ -189,6 +189,11 @@ enum e_g2d_exec_flag {
> >>>   G2D_EXEC_FLAG_ASYNC = (1 << 0)
> >>>  };
> >>>  
> >>> +enum e_g2d_dir_mode {
> >>> + G2D_DIR_MODE_POSITIVE = 0,
> >>> + G2D_DIR_MODE_NEGATIVE = 1
> >>> +};
> >>> +
> >>>  union g2d_point_val {
> >>>   unsigned int val;
> >>>   struct {
> >>> @@ -269,6 +274,39 @@ union g2d_blend_func_val {
> >>>   } data;
> >>>  };
> >>>  
> >>> +union g2d_direction_val {
>

Re: [PATCH 07/13] tests/exynos: use XRGB8888 for framebuffer

2015-10-30 Thread Hyungwon Hwang
On Tue, 22 Sep 2015 17:54:56 +0200
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> This matches the G2D color mode that is used in the entire code.
> The previous (incorrect) RGBA would only work since the
> Exynos mixer did its configuration based on the bpp, and not
> based on the actual pixelformat.
> 
> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> ---
>  tests/exynos/exynos_fimg2d_test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/exynos/exynos_fimg2d_test.c
> b/tests/exynos/exynos_fimg2d_test.c index 8794dac..dfb00a0 100644
> --- a/tests/exynos/exynos_fimg2d_test.c
> +++ b/tests/exynos/exynos_fimg2d_test.c
> @@ -675,7 +675,7 @@ int main(int argc, char **argv)
>   offsets[0] = 0;
>  
>   ret = drmModeAddFB2(dev->fd, screen_width, screen_height,
> - DRM_FORMAT_RGBA, handles,
> + DRM_FORMAT_XRGB, handles,
>       pitches, offsets, _id, 0);

Reviewed-by: Hyungwon Hwang <human.hw...@samsung.com>

Nice catch. It's right, if there was no previous setting for source
image color mode. But I think it could be the source image color mode
was set by another application before when this test runs. So I think
the code which sets the source image color mode must be added.

Best regards,
Hyungwon Hwang


>   if (ret < 0)
>   goto err_destroy_buffer;

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


Re: [PATCH 03/13] tests/exynos: add fimg2d performance analysis

2015-10-30 Thread Hyungwon Hwang
   if (h == 0) h = 1;
> +
> + img.color = rand();
> +
> + ret = g2d_solid_fill(ctx, , x, y, w, h);
> +
> + clock_gettime(CLOCK_MONOTONIC, );
> +
> + if (ret == 0)
> + ret = g2d_exec(ctx);
> +
> + if (ret != 0) {
> + fprintf(stderr, "error: iteration %u failed
> (x = %u, y = %u, w = %u, h = %u)\n",
> + i, x, y, w, h);
> + break;
> + } else {
> + struct timespec end = { 0 };
> + clock_gettime(CLOCK_MONOTONIC, );
> +
> + g2d_time = (end.tv_sec - tspec.tv_sec) *
> 10ULL;
> + g2d_time += (end.tv_nsec - tspec.tv_nsec);
> +
> + if (output_mathematica) {
> + if (i != 0) putchar(',');
> + printf("{%u,%llu}", w * h, g2d_time);
> + } else {
> + printf("num_pixels = %u, usecs =
> %llu\n", w * h, g2d_time);
> +     }
> +     }
> + }
> +
> + if (output_mathematica)
> + printf("}\n");
> +
> + return ret;
> +}
> +
> +static int fimg2d_perf_multi(struct exynos_bo *bo, struct
> g2d_context *ctx,
> + unsigned buf_width, unsigned buf_height,
> unsigned iterations, unsigned batch) +{
> + struct timespec tspec = { 0 };
> + struct g2d_image *images;
> +
> + unsigned long long g2d_time;
> + unsigned i, j;
> + int ret = 0;
> +
> + images = calloc(batch, sizeof(struct g2d_image));

I think that this should be freed at the end of this function.

Best regards,
Hyungwon Hwang

> + for (i = 0; i < batch; ++i) {
> + images[i].width = buf_width;
> + images[i].height = buf_height;
> + images[i].stride = buf_width * 4;
> + images[i].color_mode = G2D_COLOR_FMT_ARGB |
> G2D_ORDER_AXRGB;
> + images[i].buf_type = G2D_IMGBUF_GEM;
> + images[i].bo[0] = bo->handle;
> + }
> +
> + srand(time(NULL));
> +
> + printf("starting multi G2D performance test (batch size =
> %u)\n", batch);
> + printf("buffer width = %u, buffer height = %u, iterations =
> %u\n",
> + buf_width, buf_height, iterations);
> +
> + if (output_mathematica)
> + putchar('{');
> +
> + for (i = 0; i < iterations; ++i) {
> + unsigned num_pixels = 0;
> +
> + for (j = 0; j < batch; ++j) {
> + unsigned x, y, w, h;
> +
> + x = rand() % buf_width;
> + y = rand() % buf_height;
> +
> + if (x == (buf_width - 1))
> + x -= 1;
> + if (y == (buf_height - 1))
> + y -= 1;
> +
> + w = rand() % (buf_width - x);
> + h = rand() % (buf_height - y);
> +
> + if (w == 0) w = 1;
> + if (h == 0) h = 1;
> +
> + images[j].color = rand();
> +
> + num_pixels += w * h;
> +
> + ret = g2d_solid_fill(ctx, [j], x, y,
> w, h);
> + if (ret != 0)
> + break;
> + }
> +
> + clock_gettime(CLOCK_MONOTONIC, );
> +
> + if (ret == 0)
> + ret = g2d_exec(ctx);
> +
> + if (ret != 0) {
> + fprintf(stderr, "error: iteration %u failed
> (num_pixels = %u)\n", i, num_pixels);
> + break;
> + break;
> + } else {
> + struct timespec end = { 0 };
> + clock_gettime(CLOCK_MONOTONIC, );
> +
> + g2d_time = (end.tv_sec - tspec.tv_sec) *
> 10ULL;
> + g2d_time += (end.tv_nsec - tspec.tv_nsec);
> +
> + if (output_mathematica) {
> + if (i != 0) putchar(',');
> + printf("{%u,%llu}", num_pixels,
> g2d_time);
> + } else {
> + printf("num_pixels = %u, usecs =
> %llu\n", num_pixels, g2d_time);
> + }
> + }
> + }
> +
> + if (output_mathematica)
> + printf("}\n");
> +
> + return ret;
> +}
> +
>

Re: [PATCH 06/13] tests/exynos: add fimg2d event test

2015-10-30 Thread Hyungwon Hwang
On Tue, 22 Sep 2015 17:54:55 +0200
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> This tests async processing of G2D jobs. A separate thread is spawned
> to monitor the DRM fd for events and check whether a G2D job was
> completed.
> 
> v2: Add GPLv2 header, argument handling and documentation.
> Test is only installed when requested.
> v3: Allocate G2D jobs with calloc which fixes 'busy' being
> potentially uninitialized. Also enable timeout for poll()
> in the monitor thread. This fixes pthread_join() not working
> because of poll() not returning.
> 
> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> ---
>  tests/exynos/Makefile.am   |  11 +-
>  tests/exynos/exynos_fimg2d_event.c | 326
> + 2 files changed, 335
> insertions(+), 2 deletions(-) create mode 100644
> tests/exynos/exynos_fimg2d_event.c
> 
> diff --git a/tests/exynos/Makefile.am b/tests/exynos/Makefile.am
> index e82d199..357d6b8 100644
> --- a/tests/exynos/Makefile.am
> +++ b/tests/exynos/Makefile.am
> @@ -20,16 +20,23 @@ endif
>  
>  if HAVE_INSTALL_TESTS
>  bin_PROGRAMS += \
> - exynos_fimg2d_perf
> + exynos_fimg2d_perf \
> + exynos_fimg2d_event
>  else
>  noinst_PROGRAMS += \
> - exynos_fimg2d_perf
> + exynos_fimg2d_perf \
> + exynos_fimg2d_event
>  endif
>  
>  exynos_fimg2d_perf_LDADD = \
>   $(top_builddir)/libdrm.la \
>   $(top_builddir)/exynos/libdrm_exynos.la
>  
> +exynos_fimg2d_event_LDADD = \
> + $(top_builddir)/libdrm.la \
> + $(top_builddir)/exynos/libdrm_exynos.la \
> + -lpthread
> +
>  exynos_fimg2d_test_LDADD = \
>   $(top_builddir)/libdrm.la \
>   $(top_builddir)/libkms/libkms.la \
> diff --git a/tests/exynos/exynos_fimg2d_event.c
> b/tests/exynos/exynos_fimg2d_event.c new file mode 100644
> index 000..c03dcff
> --- /dev/null
> +++ b/tests/exynos/exynos_fimg2d_event.c
> @@ -0,0 +1,326 @@
> +/*
> + * Copyright (C) 2015 - Tobias Jakobi
> + *
> + * This is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> + * by the Free Software Foundation, either version 2 of the License,
> + * or (at your option) any later version.
> + *
> + * It is distributed in the hope that it will be useful, but
> + * WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + * You should have received a copy of the GNU General Public License
> + * along with it. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include 
> +
> +#include "exynos_drm.h"
> +#include "exynos_drmif.h"
> +#include "exynos_fimg2d.h"
> +
> +struct g2d_job {
> + unsigned int id;
> + unsigned int busy;
> +};
> +
> +struct exynos_evhandler {
> + struct pollfd fds;
> + struct exynos_event_context evctx;
> +};
> +
> +struct threaddata {
> + unsigned int stop;
> + struct exynos_device *dev;
> + struct exynos_evhandler evhandler;
> +};
> +
> +static void g2d_event_handler(int fd, unsigned int cmdlist_no,
> unsigned int tv_sec,
> + unsigned int
> tv_usec, void *user_data) +{
> + struct g2d_job *job = user_data;
> +
> + fprintf(stderr, "info: g2d job (id = %u, cmdlist number =
> %u) finished!\n",
> + job->id, cmdlist_no);
> +
> + job->busy = 0;
> +}
> +
> +static void setup_g2d_event_handler(struct exynos_evhandler
> *evhandler, int fd) +{
> + evhandler->fds.fd = fd;
> + evhandler->fds.events = POLLIN;
> + evhandler->evctx.base.version = DRM_EVENT_CONTEXT_VERSION;
> + evhandler->evctx.version = EXYNOS_EVENT_CONTEXT_VERSION;

The versions must be set not using XXX_EVENT_CONTEXT_VERSION. After the
versions are bumped, the event will contains wrong version info.

Also, I think the type of event must be set here.

Best regards,
Hyungwon Hwang

> + evhandler->evctx.g2d_event_handler = g2d_event_handler;
> +}
> +
> +static void* threadfunc(void *arg) {
> + const int timeout = 0;
> + struct threaddata *data;
> +
> + data = arg;
> +
> + while (1) {
> + if (data->stop) break;
> +
> + usleep(500);
> +
> + data->evhandler.fds.revents = 0;
> +
> + if (po

Re: [PATCH 08/13] exynos: fimg2d: add g2d_set_direction

2015-10-30 Thread Hyungwon Hwang
On Tue, 22 Sep 2015 17:54:57 +0200
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> This allows setting the two direction registers, which specify how
> the engine blits pixels. This can be used for overlapping blits,
> which happen e.g. when 'moving' a rectangular region inside a
> fixed buffer.

Code itself looks good. But as I know, direction registers are related
with flip, not moving. Isn't it? I am not that much familiar with G2D.
Please let me know if I am wrong.

Best regards,
Hyungwon Hwang

> 
> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> ---
>  exynos/exynos_fimg2d.c | 13 +
>  exynos/exynos_fimg2d.h | 38 ++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> index e997d4b..4d5419c 100644
> --- a/exynos/exynos_fimg2d.c
> +++ b/exynos/exynos_fimg2d.c
> @@ -242,6 +242,19 @@ static void g2d_add_base_addr(struct g2d_context
> *ctx, struct g2d_image *img, }
>  
>  /*
> + * g2d_set_direction - setup direction register (useful for
> overlapping blits).
> + *
> + * @ctx: a pointer to g2d_context structure.
> + * @dir: a pointer to the g2d_direction_val structure.
> + */
> +static void g2d_set_direction(struct g2d_context *ctx,
> + const union g2d_direction_val *dir)
> +{
> + g2d_add_cmd(ctx, SRC_MASK_DIRECT_REG, dir->val[0]);
> + g2d_add_cmd(ctx, DST_PAT_DIRECT_REG, dir->val[1]);
> +}
> +
> +/*
>   * g2d_reset - reset fimg2d hardware.
>   *
>   * @ctx: a pointer to g2d_context structure.
> diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h
> index c6b58ac..9eee7c0 100644
> --- a/exynos/exynos_fimg2d.h
> +++ b/exynos/exynos_fimg2d.h
> @@ -189,6 +189,11 @@ enum e_g2d_exec_flag {
>   G2D_EXEC_FLAG_ASYNC = (1 << 0)
>  };
>  
> +enum e_g2d_dir_mode {
> + G2D_DIR_MODE_POSITIVE = 0,
> + G2D_DIR_MODE_NEGATIVE = 1
> +};
> +
>  union g2d_point_val {
>   unsigned int val;
>   struct {
> @@ -269,6 +274,39 @@ union g2d_blend_func_val {
>   } data;
>  };
>  
> +union g2d_direction_val {
> + unsigned int val[2];
> + struct {
> + /* SRC_MSK_DIRECT_REG [0:1] (source) */
> + enum e_g2d_dir_mode src_x_direction:1;
> + enum e_g2d_dir_mode src_y_direction:1;
> +
> + /* SRC_MSK_DIRECT_REG [2:3] */
> + unsigned intreversed1:2;
> +
> + /* SRC_MSK_DIRECT_REG [4:5] (mask) */
> + enum e_g2d_dir_mode
> mask_x_direction:1;
> + enum e_g2d_dir_mode
> mask_y_direction:1; +
> + /* SRC_MSK_DIRECT_REG [6:31] */
> + unsigned intpadding1:26;
> +
> + /* DST_PAT_DIRECT_REG [0:1] (destination) */
> + enum e_g2d_dir_mode dst_x_direction:1;
> + enum e_g2d_dir_mode dst_y_direction:1;
> +
> + /* DST_PAT_DIRECT_REG [2:3] */
> + unsigned intreversed2:2;
> +
> + /* DST_PAT_DIRECT_REG [4:5] (pattern) */
> + enum e_g2d_dir_mode pat_x_direction:1;
> + enum e_g2d_dir_mode pat_y_direction:1;
> +
> + /* DST_PAT_DIRECT_REG [6:31] */
> + unsigned intpadding2:26;
> + } data;
> +};
> +
>  struct g2d_image {
>   enum e_g2d_select_mode  select_mode;
>   enum e_g2d_color_mode   color_mode;

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


Re: [PATCH 09/13] exynos/fimg2d: add g2d_move

2015-10-30 Thread Hyungwon Hwang
On Tue, 22 Sep 2015 17:54:58 +0200
Tobias Jakobi <tjak...@math.uni-bielefeld.de> wrote:

> We already have g2d_copy() which implements G2D copy
> operations from one buffer to another. However we can't
> do a overlapping copy operation in one buffer.
> 
> Add g2d_move() which acts like the standard memmove()
> and properly handles overlapping copies.
> 
> Signed-off-by: Tobias Jakobi <tjak...@math.uni-bielefeld.de>
> ---
>  exynos/exynos_fimg2d.c | 94
> ++
> exynos/exynos_fimg2d.h |  3 ++ 2 files changed, 97 insertions(+)
> 
> diff --git a/exynos/exynos_fimg2d.c b/exynos/exynos_fimg2d.c
> index 4d5419c..8703629 100644
> --- a/exynos/exynos_fimg2d.c
> +++ b/exynos/exynos_fimg2d.c
> @@ -540,6 +540,100 @@ g2d_copy(struct g2d_context *ctx, struct
> g2d_image *src, }
>  
>  /**
> + * g2d_move - move content inside single buffer.
> + *   Similar to 'memmove' this moves a rectangular region
> + *   of the provided buffer to another location (the source
> + *   and destination region potentially overlapping).
> + *
> + * @ctx: a pointer to g2d_context structure.
> + * @img: a pointer to g2d_image structure providing
> + *   buffer information.
> + * @src_x: x position of source rectangle.
> + * @src_y: y position of source rectangle.
> + * @dst_x: x position of destination rectangle.
> + * @dst_y: y position of destination rectangle.
> + * @w: width of rectangle to move.
> + * @h: height of rectangle to move.
> + */
> +int
> +g2d_move(struct g2d_context *ctx, struct g2d_image *img,
> + unsigned int src_x, unsigned int src_y,
> + unsigned int dst_x, unsigned dst_y, unsigned int w,
> + unsigned int h)
> +{
> + union g2d_rop4_val rop4;
> + union g2d_point_val pt;
> + union g2d_direction_val dir;
> + unsigned int src_w, src_h, dst_w, dst_h;
> +
> + src_w = w;
> + src_h = h;
> + if (src_x + img->width > w)
> + src_w = img->width - src_x;
> + if (src_y + img->height > h)
> + src_h = img->height - src_y;
> +
> + dst_w = w;
> + dst_h = w;
> + if (dst_x + img->width > w)
> + dst_w = img->width - dst_x;
> + if (dst_y + img->height > h)
> + dst_h = img->height - dst_y;
> +
> + w = MIN(src_w, dst_w);
> + h = MIN(src_h, dst_h);
> +
> + if (w == 0 || h == 0) {
> + fprintf(stderr, MSG_PREFIX "invalid width or
> height.\n");
> + return -EINVAL;
> + }
> +
> + if (g2d_check_space(ctx, 13, 2))
> + return -ENOSPC;
> +
> + g2d_add_cmd(ctx, DST_SELECT_REG, G2D_SELECT_MODE_BGCOLOR);
> + g2d_add_cmd(ctx, SRC_SELECT_REG, G2D_SELECT_MODE_NORMAL);
> +
> + g2d_add_cmd(ctx, DST_COLOR_MODE_REG, img->color_mode);
> + g2d_add_cmd(ctx, SRC_COLOR_MODE_REG, img->color_mode);
> +
> + g2d_add_base_addr(ctx, img, g2d_dst);
> + g2d_add_base_addr(ctx, img, g2d_src);
> +
> + g2d_add_cmd(ctx, DST_STRIDE_REG, img->stride);
> + g2d_add_cmd(ctx, SRC_STRIDE_REG, img->stride);
> +
> + dir.val[0] = dir.val[1] = 0;
> +
> + if (dst_x >= src_x)
> + dir.data.src_x_direction = dir.data.dst_x_direction
> = 1;
> + if (dst_y >= src_y)
> + dir.data.src_y_direction = dir.data.dst_y_direction
> = 1; +

As I commented in the patch 08/13, I think that direction is related
with flip. If I am right, these two if statements looks awkward.

Best regards,
Hyungwon Hwang

> + g2d_set_direction(ctx, );
> +
> + pt.data.x = src_x;
> + pt.data.y = src_y;
> + g2d_add_cmd(ctx, SRC_LEFT_TOP_REG, pt.val);
> + pt.data.x = src_x + w;
> + pt.data.y = src_y + h;
> + g2d_add_cmd(ctx, SRC_RIGHT_BOTTOM_REG, pt.val);
> +
> + pt.data.x = dst_x;
> + pt.data.y = dst_y;
> + g2d_add_cmd(ctx, DST_LEFT_TOP_REG, pt.val);
> + pt.data.x = dst_x + w;
> + pt.data.y = dst_y + h;
> + g2d_add_cmd(ctx, DST_RIGHT_BOTTOM_REG, pt.val);
> +
> + rop4.val = 0;
> + rop4.data.unmasked_rop3 = G2D_ROP3_SRC;
> + g2d_add_cmd(ctx, ROP4_REG, rop4.val);
> +
> + return g2d_flush(ctx);
> +}
> +
> +/**
>   * g2d_copy_with_scale - copy contents in source buffer to
> destination buffer
>   *   scaling up or down properly.
>   *
> diff --git a/exynos/exynos_fimg2d.h b/exynos/exynos_fimg2d.h
> index 9eee7c0..2700686 100644
> --- a/exynos/exynos_fimg2d.h
> +++ b/exynos/exynos_fimg2d.h
> @@ -343,6 +343,9 @@ int g2d_copy(struct g2d_context *ctx, struct
> g2d_image *src, struct g2d_image *dst,

Re: [PATCH] drm/exynos: iommu: fix potential NULL pointer dereference

2015-06-29 Thread Hyungwon Hwang
Dear Marek,

On Thu, 25 Jun 2015 15:10:12 +0200
Marek Szyprowski m.szyprow...@samsung.com wrote:

 Some drivers (like Exynos mixer) calls
 drm_iommu_attach_device_if_possible before registering crtc, so
 additional check for NULL crtc pointer is required.

It seems reasonable.

Reviewed-by: Hyungwon Hwang human.hw...@samsung.com

 
 Signed-off-by: Marek Szyprowski m.szyprow...@samsung.com
 ---
  drivers/gpu/drm/exynos/exynos_drm_iommu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/drivers/gpu/drm/exynos/exynos_drm_iommu.c
 b/drivers/gpu/drm/exynos/exynos_drm_iommu.c index
 d4ec7465e9cc..d4eb730ba254 100644 ---
 a/drivers/gpu/drm/exynos/exynos_drm_iommu.c +++
 b/drivers/gpu/drm/exynos/exynos_drm_iommu.c @@ -151,7 +151,7 @@ int
 drm_iommu_attach_device_if_possible(struct exynos_drm_crtc
 *exynos_crtc, int ret = 0; 
   if (is_drm_iommu_supported(drm_dev)) {
 - if (exynos_crtc-ops-clear_channels)
 + if (exynos_crtc  exynos_crtc-ops-clear_channels)
   exynos_crtc-ops-clear_channels(exynos_crtc);
   return drm_iommu_attach_device(drm_dev, subdrv_dev);
   }

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


[PATCH v3] ARM: dts: fix the clock-frequency of rinato board's panel

2015-06-14 Thread Hyungwon Hwang
From: Hyungwon Hwang human.hw...@samsung.com

After the commit abc0b1447d4974963548777a5ba4a4457c82c426
(drm: Perform basic sanity checks on probed modes), proper
clock-frequency becomes mandatory for validating the mode of panel.
The display does not work if there is no mode validated. Also, this
clock-frequency must be set appropriately for getting required frame
rate.

Fixes: abc0b1447d49 (drm: Perform basic sanity checks on probed modes)
Cc: sta...@vger.kernel.org
Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts 
b/arch/arm/boot/dts/exynos3250-rinato.dts
index 0b99068..75aba40 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -181,7 +181,7 @@
 
display-timings {
timing-0 {
-   clock-frequency = 0;
+   clock-frequency = 460;
hactive = 320;
vactive = 320;
hfront-porch = 1;
-- 
1.8.1.2

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


[PATCH 2/3] ARM: dts: fix the clock-frequency of rinato board's panel

2015-06-12 Thread Hyungwon Hwang
Because of recent update, proper clock-frequency becomes mandatory
for validating the mode of panel. This clock-frequency must be set
appropriately for getting required frame rate.

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts 
b/arch/arm/boot/dts/exynos3250-rinato.dts
index 0b99068..75aba40 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -181,7 +181,7 @@
 
display-timings {
timing-0 {
-   clock-frequency = 0;
+   clock-frequency = 460;
hactive = 320;
vactive = 320;
hfront-porch = 1;
-- 
1.9.1

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


Re: [PATCH 2/3] ARM: dts: fix the clock-frequency of rinato board's panel

2015-06-12 Thread Hyungwon Hwang
On Fri, 12 Jun 2015 18:23:18 +0900
Krzysztof Kozlowski k.kozlow...@samsung.com wrote:

 2015-06-12 18:03 GMT+09:00 Hyungwon Hwang human.hw...@samsung.com:
  Because of recent update, proper clock-frequency becomes mandatory
  for validating the mode of panel. This clock-frequency must be set
  appropriately for getting required frame rate.
 
  Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
 
 Acked-by: Krzysztof Kozlowski k.kozlow...@samsung.com
 
 Is it a bugfix? The property is mandatory right now?

With recent kernel, yes. It is mandatory for the mode to be become
available.

Best regards,
Hyungwon Hwang

 
 Best regards,
 Krzysztof

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


[PATCH 1/3] ARM: dts: Add the reference node for syscon to mipi phy for Exynos3250

2015-06-12 Thread Hyungwon Hwang
Exynos mipi phy driver needs syscon node to be probed successfully.

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 arch/arm/boot/dts/exynos3250.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos3250.dtsi 
b/arch/arm/boot/dts/exynos3250.dtsi
index e3bfb11..f8c02dd 100644
--- a/arch/arm/boot/dts/exynos3250.dtsi
+++ b/arch/arm/boot/dts/exynos3250.dtsi
@@ -140,6 +140,7 @@
compatible = samsung,s5pv210-mipi-video-phy;
reg = 0x10020710 8;
#phy-cells = 1;
+   syscon = pmu_system_controller;
};
 
pd_cam: cam-power-domain@10023C00 {
-- 
1.9.1

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


[PATCH v2] ARM: dts: fix the clock-frequency of rinato board's panel

2015-06-12 Thread Hyungwon Hwang
After the commit abc0b1447d4974963548777a5ba4a4457c82c426
(drm: Perform basic sanity checks on probed modes), proper
clock-frequency becomes mandatory for validating the mode of panel.
The display does not work if there is no mode validated. Also, this
clock-frequency must be set appropriately for getting required frame
rate.

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 arch/arm/boot/dts/exynos3250-rinato.dts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts 
b/arch/arm/boot/dts/exynos3250-rinato.dts
index 0b99068..75aba40 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -181,7 +181,7 @@

display-timings {
timing-0 {
-   clock-frequency = 0;
+   clock-frequency = 460;
hactive = 320;
vactive = 320;
hfront-porch = 1;
--
1.9.1

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


Re: [PATCH 3/3] drm/exynos: remove SoC checking code

2015-06-09 Thread Hyungwon Hwang
Hi Andrzej,

On Tue, 09 Jun 2015 16:46:52 -0300
Gustavo Padovan gust...@padovan.org wrote:

 Hi Andrzej,
 
 2015-06-08 Andrzej Hajda a.ha...@samsung.com:
 
  SoC checking code is not necessary anymore, as exynos_drm_match_add
  and exynos_drm_platform_probe already properly handles situation
  when there are no Exynos DRM components.
  
  Signed-off-by: Andrzej Hajda a.ha...@samsung.com
  ---
   drivers/gpu/drm/exynos/exynos_drm_drv.c | 27
  +-- 1 file changed, 1 insertion(+), 26
  deletions(-)
 
 This series looks goods to me and works fine on my snow machine:
 
 Tested-by: Gustavo Padovan gustavo.pado...@collabora.co.uk
 
   Gustavo

It looks good to me.

Reviewed-by: Hyungwon Hwang human.hw...@samsung.com

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

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


[PATCH 2/2] drm/exynos: ipp: validate a GEM handle with multiple planes

2015-06-08 Thread Hyungwon Hwang
FIMC  GSC driver can calculate the offset of planes. So there are
use cases which IPP receives just one GEM handle of an image with
multiple plane. This patch extends ipp_validate_mem_node() to validate
this case.

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 drivers/gpu/drm/exynos/exynos_drm_ipp.c | 51 -
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c 
b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index 54c5cf4..b3dc778 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -482,8 +482,8 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
 {
struct drm_exynos_ipp_config *ipp_cfg;
unsigned int num_plane;
-   unsigned long min_size, size;
-   unsigned int bpp;
+   unsigned long size, buf_size = 0, plane_size, img_size = 0;
+   unsigned int bpp, width, height;
int i;
 
ipp_cfg = c_node-property.config[m_node-ops_id];
@@ -497,20 +497,45 @@ static int ipp_validate_mem_node(struct drm_device 
*drm_dev,
 * but it seems more than enough
 */
for (i = 0; i  num_plane; ++i) {
-   if (!m_node-buf_info.handles[i]) {
-   DRM_ERROR(invalid handle for plane %d\n, i);
-   return -EINVAL;
-   }
+   width = ipp_cfg-sz.hsize;
+   height = ipp_cfg-sz.vsize;
bpp = drm_format_plane_cpp(ipp_cfg-fmt, i);
-   min_size = (ipp_cfg-sz.hsize * ipp_cfg-sz.vsize * bpp)  3;
-   size = exynos_drm_gem_get_size(drm_dev,
-  m_node-buf_info.handles[i],
-  c_node-filp);
-   if (min_size  size) {
-   DRM_ERROR(invalid size for plane %d\n, i);
-   return -EINVAL;
+
+   /*
+* The result of drm_format_plane_cpp() for chroma planes must
+* be used with drm_format__chroma_subsampling() for
+* correct result.
+*/
+   if (i  0) {
+   width /= drm_format_horz_chroma_subsampling(
+   ipp_cfg-fmt);
+   height /= drm_format_vert_chroma_subsampling(
+   ipp_cfg-fmt);
}
+   plane_size = width * height * bpp;
+   img_size += plane_size;
+
+   if (m_node-buf_info.handles[i]) {
+   size = exynos_drm_gem_get_size(drm_dev,
+   m_node-buf_info.handles[i],
+   c_node-filp);
+   if (plane_size  size) {
+   DRM_ERROR(
+   buffer %d is smaller than required\n,
+   i);
+   return -EINVAL;
+   }
+
+   buf_size += size;
+   }
+   }
+
+   if (buf_size  img_size) {
+   DRM_ERROR(size of buffers(%lu) is smaller than image(%lu)\n,
+   buf_size, img_size);
+   return -EINVAL;
}
+
return 0;
 }
 
-- 
1.9.1

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


[PATCH 1/2] drm/exynos: ipp: fix wrong index referencing a config element

2015-06-08 Thread Hyungwon Hwang
Config depends on the opreation. So it must be referenced by an
operation id, not a property id.

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 drivers/gpu/drm/exynos/exynos_drm_ipp.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_ipp.c 
b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
index b7f1cbc..54c5cf4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_ipp.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_ipp.c
@@ -486,8 +486,7 @@ static int ipp_validate_mem_node(struct drm_device *drm_dev,
unsigned int bpp;
int i;
 
-   /* The property id should already be varified */
-   ipp_cfg = c_node-property.config[m_node-prop_id];
+   ipp_cfg = c_node-property.config[m_node-ops_id];
num_plane = drm_format_num_planes(ipp_cfg-fmt);
 
/**
-- 
1.9.1

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


[PATCH v3] drm/exynos: dsi: check whether dsi is enabled before sending data

2015-06-08 Thread Hyungwon Hwang
exynos_dsi_host_transfer() can be called through a panel driver while
DSI is turning down. It is possible because the function checks only
whether DSI is initialized or not, and there is a moment which DSI is
set by uninitialized, but DSI is still turning down. To prevent it,
DSI must be set by disabled before starting to be turned down, and
exynos_dsi_host_transfer() must check whether DSI is enabled or not.

Kernel dump:
[ 4721.351448] Unhandled fault: synchronous external abort (0x96000210) at 
0xff800015e018
[ 4721.351809] Internal error: : 96000210 [#1] PREEMPT SMP
[ 4721.352031] Modules linked in:
[ 4721.352173] CPU: 2 PID: 300 Comm: deviced Tainted: GW   
4.0.4-01017-g7964a87 #1
[ 4721.353989] Hardware name: Samsung DRACO board (DT)
[ 4721.358852] task: ffc0a0b7 ti: ffc0a00ec000 task.ti: 
ffc0a00ec000
[ 4721.366327] PC is at exynos_dsi_enable_lane+0x14/0x5c
[ 4721.371353] LR is at exynos_dsi_host_transfer+0x834/0x8d8
[ 4721.376731] pc : [ffc000432bcc] lr : [ffc000434590] pstate: 
6145
[ 4721.384107] sp : ffc0a00efbe0
[ 4721.387405] x29: ffc0a00efbe0 x28: ffc0a00ec000
[ 4721.392699] x27: ffc000968000 x26: 0040
[ 4721.397994] x25: ffc000f74dc0 x24: ffc0a00efec8
[ 4721.403290] x23: ffc0a4815400 x22: ffc0009f2729
[ 4721.408584] x21: ffc0a00efcc8 x20: ffc0a4a2a848
[ 4721.413879] x19: ffc0a4a2a818 x18: 0004
[ 4721.419173] x17: 007faa5cddf0 x16: ffc0001a40a8
[ 4721.424469] x15: 0009 x14: 000d
[ 4721.429762] x13: 6e6e6f63206b726f x12: 0010
[ 4721.435058] x11: 0101010101010101 x10: 
[ 4721.440353] x9 : 000a x8 : 8386838282818381
[ 4721.445648] x7 : ffc0a201efe8 x6 : 
[ 4721.450943] x5 : fffa x4 : ffc0a201f170
[ 4721.456237] x3 : ff800015e000 x2 : ff800015e018
[ 4721.461531] x1 : 000f x0 : ffc0a4a2a818
[ 4721.466826]
[ 4721.468305] Process deviced (pid: 300, stack limit = 0xffc0a00ec028)
[ 4721.474989] Stack: (0xffc0a00efbe0 to 0xffc0a00f)
[ 4721.480720] fbe0: a00efca0 ffc0 0042c944 ffc0 a0f2d680 ffc0 
0024 
[ 4721.488895] fc00: a4b6d000 ffc0 009f2729 ffc0 a4815400 ffc0 
a00efec8 ffc0

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
Changes for v2:
 - Previous version of this patch makes a problem in initializing the DSI. This
   patch fixes it, by moving the point which DSI is set by enabled to the
   point before drm_panel_prepare() called. This is where the setting must
   be done, because to call drm_panel_prepare() successfully, DSI must be
   enabled. Also this patch adds the condition to TE irq handler. DSI must be
   enabled and initialized, not just enabled before calling te_handler in
   the display driver.
Changes for v3:
 - New state DSIM_STATE_VIDOUT_AVAILABLE for representing video output
   availability is introduced. Because DSIM_STATE_ENABLED becomes to
   represent whether DSI can be used for data transfer or not, this
   state can be used for checking whether video ouput is available or
   not anymore. So new state have to be introduced. The stete
   DSIM_STATE_VIDOUT_AVAILABLE represents whether DSI is prepared for
   outputting video to a panel or not.

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 1cfc4be07..9250356 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -241,6 +241,7 @@ struct exynos_dsi_transfer {
 #define DSIM_STATE_ENABLED BIT(0)
 #define DSIM_STATE_INITIALIZED BIT(1)
 #define DSIM_STATE_CMD_LPM BIT(2)
+#define DSIM_STATE_VIDOUT_AVAILABLEBIT(3)

 struct exynos_dsi_driver_data {
unsigned int *regs;
@@ -1268,7 +1269,7 @@ static irqreturn_t exynos_dsi_te_irq_handler(int irq, 
void *dev_id)
struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
struct drm_encoder *encoder = dsi-display.encoder;

-   if (dsi-state  DSIM_STATE_ENABLED)
+   if (dsi-state  DSIM_STATE_VIDOUT_AVAILABLE)
exynos_drm_crtc_te_handler(encoder-crtc);

return IRQ_HANDLED;
@@ -1408,6 +1409,9 @@ static ssize_t exynos_dsi_host_transfer(struct 
mipi_dsi_host *host,
struct exynos_dsi_transfer xfer;
int ret;

+   if (!(dsi-state  DSIM_STATE_ENABLED))
+   return -EINVAL;
+
if (!(dsi-state  DSIM_STATE_INITIALIZED)) {
ret = exynos_dsi_init(dsi);
if (ret)
@@ -1520,8 +1524,11 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
if (ret  0)
return ret;

+   dsi-state |= DSIM_STATE_ENABLED;
+
ret = drm_panel_prepare(dsi-panel);
if (ret  0) {
+   dsi-state = ~DSIM_STATE_ENABLED

[PATCH v2] drm/exynos: dsi: check whether dsi is enabled before sending data

2015-06-07 Thread Hyungwon Hwang
exynos_dsi_host_transfer() can be called through a panel driver while
DSI is turning down. It is possible because the function checks only
whether DSI is initialized or not, and there is a moment which DSI is
set by uninitialized, but DSI is still turning down. To prevent it,
DSI must be set by disabled before starting to be turned down, and
exynos_dsi_host_transfer() must check whether DSI is enabled or not.

Kernel dump:
[ 4721.351448] Unhandled fault: synchronous external abort (0x96000210) at 
0xff800015e018
[ 4721.351809] Internal error: : 96000210 [#1] PREEMPT SMP
[ 4721.352031] Modules linked in:
[ 4721.352173] CPU: 2 PID: 300 Comm: deviced Tainted: GW   
4.0.4-01017-g7964a87 #1
[ 4721.353989] Hardware name: Samsung DRACO board (DT)
[ 4721.358852] task: ffc0a0b7 ti: ffc0a00ec000 task.ti: 
ffc0a00ec000
[ 4721.366327] PC is at exynos_dsi_enable_lane+0x14/0x5c
[ 4721.371353] LR is at exynos_dsi_host_transfer+0x834/0x8d8
[ 4721.376731] pc : [ffc000432bcc] lr : [ffc000434590] pstate: 
6145
[ 4721.384107] sp : ffc0a00efbe0
[ 4721.387405] x29: ffc0a00efbe0 x28: ffc0a00ec000
[ 4721.392699] x27: ffc000968000 x26: 0040
[ 4721.397994] x25: ffc000f74dc0 x24: ffc0a00efec8
[ 4721.403290] x23: ffc0a4815400 x22: ffc0009f2729
[ 4721.408584] x21: ffc0a00efcc8 x20: ffc0a4a2a848
[ 4721.413879] x19: ffc0a4a2a818 x18: 0004
[ 4721.419173] x17: 007faa5cddf0 x16: ffc0001a40a8
[ 4721.424469] x15: 0009 x14: 000d
[ 4721.429762] x13: 6e6e6f63206b726f x12: 0010
[ 4721.435058] x11: 0101010101010101 x10: 
[ 4721.440353] x9 : 000a x8 : 8386838282818381
[ 4721.445648] x7 : ffc0a201efe8 x6 : 
[ 4721.450943] x5 : fffa x4 : ffc0a201f170
[ 4721.456237] x3 : ff800015e000 x2 : ff800015e018
[ 4721.461531] x1 : 000f x0 : ffc0a4a2a818
[ 4721.466826]
[ 4721.468305] Process deviced (pid: 300, stack limit = 0xffc0a00ec028)
[ 4721.474989] Stack: (0xffc0a00efbe0 to 0xffc0a00f)
[ 4721.480720] fbe0: a00efca0 ffc0 0042c944 ffc0 a0f2d680 ffc0 
0024 
[ 4721.488895] fc00: a4b6d000 ffc0 009f2729 ffc0 a4815400 ffc0 
a00efec8 ffc0

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
Changes for v2:
 - Previous version of this patch makes a problem in initializing the DSI. This
   patch fixes it, by moving the point which DSI is set by enabled to the
   point before drm_panel_prepare() called. This is where the setting must
   be done, because to call drm_panel_prepare() successfully, DSI must be
   enabled. Also this patch adds the condition to TE irq handler. DSI must be
   enabled and initialized, not just enabled before calling te_handler in
   the display driver.

 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 1cfc4be07..a2ca956 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1268,7 +1268,8 @@ static irqreturn_t exynos_dsi_te_irq_handler(int irq, 
void *dev_id)
struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
struct drm_encoder *encoder = dsi-display.encoder;

-   if (dsi-state  DSIM_STATE_ENABLED)
+   if ((dsi-state  DSIM_STATE_ENABLED) 
+   (dsi-state  DSIM_STATE_INITIALIZED))
exynos_drm_crtc_te_handler(encoder-crtc);

return IRQ_HANDLED;
@@ -1408,6 +1409,9 @@ static ssize_t exynos_dsi_host_transfer(struct 
mipi_dsi_host *host,
struct exynos_dsi_transfer xfer;
int ret;

+   if (!(dsi-state  DSIM_STATE_ENABLED))
+   return -EINVAL;
+
if (!(dsi-state  DSIM_STATE_INITIALIZED)) {
ret = exynos_dsi_init(dsi);
if (ret)
@@ -1520,8 +1524,11 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
if (ret  0)
return ret;

+   dsi-state |= DSIM_STATE_ENABLED;
+
ret = drm_panel_prepare(dsi-panel);
if (ret  0) {
+   dsi-state = ~DSIM_STATE_ENABLED;
exynos_dsi_poweroff(dsi);
return ret;
}
@@ -1529,8 +1536,6 @@ static int exynos_dsi_enable(struct exynos_dsi *dsi)
exynos_dsi_set_display_mode(dsi);
exynos_dsi_set_display_enable(dsi, true);

-   dsi-state |= DSIM_STATE_ENABLED;
-
ret = drm_panel_enable(dsi-panel);
if (ret  0) {
dsi-state = ~DSIM_STATE_ENABLED;
@@ -1551,9 +1556,10 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
drm_panel_disable(dsi-panel);
exynos_dsi_set_display_enable(dsi, false);
drm_panel_unprepare(dsi-panel);
-   exynos_dsi_poweroff(dsi);

dsi-state = ~DSIM_STATE_ENABLED

[PATCH] drm/exynos: dsi: check whether dsi is enabled before sending data

2015-06-04 Thread Hyungwon Hwang
exynos_dsi_host_transfer() can be called through a panel driver while
DSI is turning down. It is possible because the function checks only
whether DSI is initialized or not, and there is a moment which DSI is
set by uninitialized, but DSI is still turning down. To prevent it,
DSI must be set by disabled before starting to be turned down, and
exynos_dsi_host_transfer() must check whether DSI is enabled or not.

Kernel dump:
[ 4721.351448] Unhandled fault: synchronous external abort (0x96000210) at 
0xff800015e018
[ 4721.351809] Internal error: : 96000210 [#1] PREEMPT SMP
[ 4721.352031] Modules linked in:
[ 4721.352173] CPU: 2 PID: 300 Comm: deviced Tainted: GW   
4.0.4-01017-g7964a87 #1
[ 4721.353989] Hardware name: Samsung DRACO board (DT)
[ 4721.358852] task: ffc0a0b7 ti: ffc0a00ec000 task.ti: 
ffc0a00ec000
[ 4721.366327] PC is at exynos_dsi_enable_lane+0x14/0x5c
[ 4721.371353] LR is at exynos_dsi_host_transfer+0x834/0x8d8
[ 4721.376731] pc : [ffc000432bcc] lr : [ffc000434590] pstate: 
6145
[ 4721.384107] sp : ffc0a00efbe0
[ 4721.387405] x29: ffc0a00efbe0 x28: ffc0a00ec000
[ 4721.392699] x27: ffc000968000 x26: 0040
[ 4721.397994] x25: ffc000f74dc0 x24: ffc0a00efec8
[ 4721.403290] x23: ffc0a4815400 x22: ffc0009f2729
[ 4721.408584] x21: ffc0a00efcc8 x20: ffc0a4a2a848
[ 4721.413879] x19: ffc0a4a2a818 x18: 0004
[ 4721.419173] x17: 007faa5cddf0 x16: ffc0001a40a8
[ 4721.424469] x15: 0009 x14: 000d
[ 4721.429762] x13: 6e6e6f63206b726f x12: 0010
[ 4721.435058] x11: 0101010101010101 x10: 
[ 4721.440353] x9 : 000a x8 : 8386838282818381
[ 4721.445648] x7 : ffc0a201efe8 x6 : 
[ 4721.450943] x5 : fffa x4 : ffc0a201f170
[ 4721.456237] x3 : ff800015e000 x2 : ff800015e018
[ 4721.461531] x1 : 000f x0 : ffc0a4a2a818
[ 4721.466826]
[ 4721.468305] Process deviced (pid: 300, stack limit = 0xffc0a00ec028)
[ 4721.474989] Stack: (0xffc0a00efbe0 to 0xffc0a00f)
[ 4721.480720] fbe0: a00efca0 ffc0 0042c944 ffc0 a0f2d680 ffc0 
0024 
[ 4721.488895] fc00: a4b6d000 ffc0 009f2729 ffc0 a4815400 ffc0 
a00efec8 ffc0

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c 
b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 1cfc4be07..6413339 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -1408,6 +1408,9 @@ static ssize_t exynos_dsi_host_transfer(struct 
mipi_dsi_host *host,
struct exynos_dsi_transfer xfer;
int ret;
 
+   if (!(dsi-state  DSIM_STATE_ENABLED))
+   return -EINVAL;
+
if (!(dsi-state  DSIM_STATE_INITIALIZED)) {
ret = exynos_dsi_init(dsi);
if (ret)
@@ -1551,9 +1554,10 @@ static void exynos_dsi_disable(struct exynos_dsi *dsi)
drm_panel_disable(dsi-panel);
exynos_dsi_set_display_enable(dsi, false);
drm_panel_unprepare(dsi-panel);
-   exynos_dsi_poweroff(dsi);
 
dsi-state = ~DSIM_STATE_ENABLED;
+
+   exynos_dsi_poweroff(dsi);
 }
 
 static void exynos_dsi_dpms(struct exynos_drm_display *display, int mode)
-- 
1.9.1

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


Re: odroidxu3 u-boot samsung - git - Denx

2015-02-08 Thread Hyungwon Hwang
Hi,

On Sun, 08 Feb 2015 10:51:55 + (UTC)
Anand Moon moon.li...@yahoo.com wrote:

 Hi All,
 
 I am trying to build upstream  the u-boot from the 
 git://git.denx.de/u-boot-samsung.git
 
 I checkout the git repository.
 
 to build the u-boot-dts.bin, I used the following commands.
 
 # make odroid-xu3_config
 # make
 
 After the I copied the u-boot-dts.bin to sd_fuse/hardkernel/.
 
 After that I issued the command 
 
 sudo ./sd_fusing.sh /dev/sdb# Which loaded the boot loader on
 the /dev/sdb
 
 After these changes I could not issue or set any further command onto
 the Odroid XU3 # command.

sd_fusing.sh script write the bootloader to the SD card which is
connected to your PC. It is irrelevant to your Odroid machine. I think
you may find more helpful information from odroid forum.

 
 So I pulled some flags changes from the
 #u-boot-samsung/include/configs/odroid.h to
 #u-boot-samsung/include/configs/odroid_xu3.h
 
 
 After these change I am able to set the environment variables for
 u-boot. But I am not able to boot into new kernel. Please some one
 could help me guide me If I am doing some thing wrong.
 

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


Re: [PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

2015-01-26 Thread Hyungwon Hwang
Dear Emil,

On Mon, 26 Jan 2015 19:05:51 +
Emil Velikov emil.l.veli...@gmail.com wrote:

 On 26 January 2015 at 01:42, Hyungwon Hwang human.hw...@samsung.com
 wrote:
  Dear Tobias,
 
  Thanks for fixing it.
 
  Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
 
 Hi Hyungwon Hwang
 
 I'm a bit confused about the use of s-o-b here. I've assumed that
 libdrm follows the kernel style [1] for those type of things ?
 Would you have anything handy which I can read on the topic ?

I also think that DRI-devel mailing list follow the kernel rule. I used
s-o-b here by the meaning of (a) below (this text is excerpted from the
link you sent)

12) Sign your work

To improve tracking of who did what, especially with patches that can
percolate to their final resting place in the kernel through several
layers of maintainers, we've introduced a sign-off procedure on
patches that are being emailed around.

The sign-off is a simple line at the end of the explanation for the
patch, which certifies that you wrote it or otherwise have the right to
pass it on as an open-source patch.  The rules are pretty simple: if you
can certify the below:

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

Was it wrong to add s-o-b here? Should I have used Reviewed-by here?
I am sorry, if I broke the rule and confused you.

 
 Cheers,
 Emil
 
 [1]
 https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/tree/Documentation/SubmittingPatches#n358
 
 A: Because it messes up the order in which people normally read text.
 Q: Why is top-posting such a bad thing?


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


Re: [PATCH 1/2] exynos: Don't use DRM_EXYNOS_GEM_{MAP_OFFSET/MMAP} ioctls

2015-01-25 Thread Hyungwon Hwang
Dear Tobias,

Thanks for fixing it.

Signed-off-by: Hyungwon Hwang human.hw...@samsung.com

On Fri, 16 Jan 2015 23:57:33 +0100
Tobias Jakobi tjak...@math.uni-bielefeld.de wrote:

 From: Hyungwon Hwang human.hw...@samsung.com
 
 The ioctl DRM_EXYNOS_GEM_MAP_OFFSET and DRM_EXYNOS_GEM_MMAP are
 removed from the linux kernel. This patch modifies libdrm and libkms
 to use drm generic ioctls instead of the removed ioctls.
 
 v2: The original patch was erroneous. In case the MODE_MAP_DUMB ioctl
 failed it would return the retvalue as a void-pointer. Users of
 libdrm would then happily use that ptr, eventually leading to a
 segfault. Change this to return NULL in that case and also restore
 the previous behaviour of logging to stderr.
 The other error was that 'bo-vaddr' was never filled with the
 mapped buffer address. Hence exynos_bo_map still returned NULL even
 if the buffer mapping succeeded.
 
 Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
 Signed-off-by: Inki Dae inki@samsung.com
 Signed-off-by: Tobias Jakobi tjak...@math.uni-bielefeld.de
 ---
  exynos/exynos_drm.c | 19 ---
  libkms/exynos.c |  7 ---
  2 files changed, 16 insertions(+), 10 deletions(-)
 
 diff --git a/exynos/exynos_drm.c b/exynos/exynos_drm.c
 index 4c7dd13..c5dd948 100644
 --- a/exynos/exynos_drm.c
 +++ b/exynos/exynos_drm.c
 @@ -283,20 +283,25 @@ drm_public void *exynos_bo_map(struct exynos_bo
 *bo) {
   if (!bo-vaddr) {
   struct exynos_device *dev = bo-dev;
 - struct drm_exynos_gem_mmap req = {
 - .handle = bo-handle,
 - .size   = bo-size,
 - };
 + struct drm_mode_map_dumb arg;
 + void *map = NULL;
   int ret;
  
 - ret = drmIoctl(dev-fd, DRM_IOCTL_EXYNOS_GEM_MMAP,
 req);
 + memset(arg, 0, sizeof(arg));
 + arg.handle = bo-handle;
 +
 + ret = drmIoctl(dev-fd, DRM_IOCTL_MODE_MAP_DUMB,
 arg); if (ret) {
 - fprintf(stderr, failed to mmap[%s].\n,
 + fprintf(stderr, failed to map dumb
 buffer[%s].\n, strerror(errno));
   return NULL;
   }
  
 - bo-vaddr = (void *)(uintptr_t)req.mapped;
 + map = drm_mmap(0, bo-size, PROT_READ | PROT_WRITE,
 MAP_SHARED,
 + dev-fd, arg.offset);
 +
 + if (map != MAP_FAILED)
 + bo-vaddr = map;
   }
  
   return bo-vaddr;
 diff --git a/libkms/exynos.c b/libkms/exynos.c
 index 92e329c..1123482 100644
 --- a/libkms/exynos.c
 +++ b/libkms/exynos.c
 @@ -25,6 +25,7 @@
  #include sys/ioctl.h
  #include xf86drm.h
  
 +#include libdrm.h
  #include exynos_drm.h
  
  struct exynos_bo
 @@ -124,7 +125,7 @@ static int
  exynos_bo_map(struct kms_bo *_bo, void **out)
  {
   struct exynos_bo *bo = (struct exynos_bo *)_bo;
 - struct drm_exynos_gem_map_off arg;
 + struct drm_mode_map_dumb arg;
   void *map = NULL;
   int ret;
  
 @@ -137,11 +138,11 @@ exynos_bo_map(struct kms_bo *_bo, void **out)
   memset(arg, 0, sizeof(arg));
   arg.handle = bo-base.handle;
  
 - ret = drmCommandWriteRead(bo-base.kms-fd,
 DRM_EXYNOS_GEM_MAP_OFFSET, arg, sizeof(arg));
 + ret = drmIoctl(bo-base.kms-fd, DRM_IOCTL_MODE_MAP_DUMB,
 arg); if (ret)
   return ret;
  
 - map = mmap(0, bo-base.size, PROT_READ | PROT_WRITE,
 MAP_SHARED, bo-base.kms-fd, arg.offset);
 + map = drm_mmap(0, bo-base.size, PROT_READ | PROT_WRITE,
 MAP_SHARED, bo-base.kms-fd, arg.offset); if (map == MAP_FAILED)
   return -errno;
  

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


Re: [PATCH v2 2/3] drm/panel: add s6e63j0x03 LCD panel driver

2015-01-05 Thread Hyungwon Hwang
Dear all,

On Tue, 06 Jan 2015 00:21:22 +0900
Inki Dae inki@samsung.com wrote:

 On 2015년 01월 05일 23:19, Thierry Reding wrote:
  On Wed, Dec 31, 2014 at 07:41:43PM +0900, Inki Dae wrote:
  Hi Thierry,
 
  Ping~.
 
  Or is it ok to pick up this patch to my tree, exynos-drm-next? It
  doesn't seem to care for a long time.
  
  I don't seem to have a copy of the v2 2/3 patch. All I found in my
  inbox is the v2 0/3 cover-letter. Please resend with me in Cc so
  that I can properly review (and apply) the patch.
 
 Please, refer to below link,
 http://lists.freedesktop.org/archives/dri-devel/2014-December/073666.html
 
 We already sent it with you in Cc. Anyway, I will resend it.
 

Also you can find the whole patchset from

[1/3] https://patchwork.kernel.org/patch/5461421/
[2/3] https://patchwork.kernel.org/patch/5461431/
[3/3] https://patchwork.kernel.org/patch/5461441/

 Thanks,
 Inki Dae
 
  
  Thierry
  
 

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


[PATCH v2 3/3] ARM: dts: add Panel device support for exynos3250-rinato

2014-12-09 Thread Hyungwon Hwang
From: Inki Dae inki@samsung.com

This patch adds MIPI-DSI and MIPI-DSI based S6E63J0X03 AMOLED panel
device nodes for Exynos3250 Rinato board.

Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
---
Changes for v2:
- None

 arch/arm/boot/dts/exynos3250-rinato.dts | 60 +
 1 file changed, 60 insertions(+)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts 
b/arch/arm/boot/dts/exynos3250-rinato.dts
index 79aa916..a8caee5 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -125,6 +125,66 @@
};
 };
 
+dsi_0 {
+   vddcore-supply = ldo6_reg;
+   vddio-supply = ldo6_reg;
+   samsung,pll-clock-frequency = 2400;
+   status = okay;
+
+   ports {
+   #address-cells = 1;
+   #size-cells = 0;
+
+   port@1 {
+   reg = 1;
+
+   dsi_out: endpoint {
+   remote-endpoint = dsi_in;
+   samsung,burst-clock-frequency = 25000;
+   samsung,esc-clock-frequency = 2000;
+   };
+   };
+   };
+
+   panel@0 {
+   compatible = samsung,s6e63j0x03;
+   reg = 0;
+   vdd3-supply = ldo16_reg;
+   vci-supply = ldo20_reg;
+   reset-gpios = gpe0 1 0;
+   te-gpios = gpx0 6 0;
+   power-on-delay= 30;
+   power-off-delay= 120;
+   reset-delay = 5;
+   init-delay = 100;
+   flip-horizontal;
+   flip-vertical;
+   panel-width-mm = 29;
+   panel-height-mm = 29;
+
+
+   display-timings {
+   timing-0 {
+   clock-frequency = 0;
+   hactive = 320;
+   vactive = 320;
+   hfront-porch = 1;
+   hback-porch = 1;
+   hsync-len = 1;
+   vfront-porch = 150;
+   vback-porch = 1;
+   vsync-len = 2;
+   };
+   };
+
+   port {
+   dsi_in: endpoint {
+   remote-endpoint = dsi_out;
+   };
+   };
+   };
+};
+
 fimd {
status = okay;
 
-- 
1.9.1

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


[PATCH v2 0/3] add display support for exynos3250 rinato board

2014-12-09 Thread Hyungwon Hwang
This is v2 of the patchset adding support for s6e63j0x03 lcd panel.

Inki Dae sent this patchset before. Because of his busy work at company,
I modifies some point according to the review by Thierry Reding on
behalf of him.

This patch series adds Display support for exynos3250 Rinato board.
For this, it adds fimd, MIPI-DSI and Panel device nodes to
exynos3250-rinato dts file, and adds a s6e63j0x03 Amoled panel device
driver which is based on MIPI-DSI bus.

Changes for v2:
- Change the gamma table to 2-dimensional array
- Change the way to make index for brightness
- Make command functions to an array so that it can be called simply
- Change command id for reading device ID
- Change the way to handle the error condition
- Remove power variable, and use the same name variable in bl_dev
- Add the state FB_BLANK_NORMAL to represent the state which the panel
is working but blanked
- Miscellaneous changes to increase the readability and follow the
  coding-style standard

Inki Dae (3):
  ARM: dts: add fimd device support for exynos3250-rinato
  drm/panel: add s6e63j0x03 LCD panel driver
  ARM: dts: add Panel device support for exynos3250-rinato

 arch/arm/boot/dts/exynos3250-rinato.dts  |  71 
 drivers/gpu/drm/panel/Kconfig|   6 +
 drivers/gpu/drm/panel/Makefile   |   1 +
 drivers/gpu/drm/panel/panel-s6e63j0x03.c | 549 +++
 4 files changed, 627 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-s6e63j0x03.c

-- 
1.9.1

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


[PATCH v2 1/3] ARM: dts: add fimd device support for exynos3250-rinato

2014-12-09 Thread Hyungwon Hwang
From: Inki Dae inki@samsung.com

This patch adds fimd device node which is a display controller
for Exynos3250 Rinato board.

Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
---
Changes for v2:
- None

 arch/arm/boot/dts/exynos3250-rinato.dts | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm/boot/dts/exynos3250-rinato.dts 
b/arch/arm/boot/dts/exynos3250-rinato.dts
index 80aa8b4..79aa916 100644
--- a/arch/arm/boot/dts/exynos3250-rinato.dts
+++ b/arch/arm/boot/dts/exynos3250-rinato.dts
@@ -125,6 +125,17 @@
};
 };
 
+fimd {
+   status = okay;
+
+   i80-if-timings {
+   cs-setup = 0;
+   wr-setup = 0;
+   wr-act = 1;
+   wr-hold = 0;
+   };
+};
+
 i2c_0 {
#address-cells = 1;
#size-cells = 0;
-- 
1.9.1

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


[PATCH v2 2/3] drm/panel: add s6e63j0x03 LCD panel driver

2014-12-09 Thread Hyungwon Hwang
From: Inki Dae inki@samsung.com

This patch adds MIPI-DSI based S6E63J0X03 AMOLED LCD panel driver
which uses mipi_dsi bus to communicate with panel. The panel has
320×320 resolution in 1.63-inch physical panel. This panel is used in
Samsung Galaxy Gear 2.

Signed-off-by: Inki Dae inki@samsung.com
Signed-off-by: Hyungwon Hwang human.hw...@samsung.com
Acked-by: Kyungmin Park kyungmin.p...@samsung.com
---
Changes for v2:
- Change the gamma table to 2-dimensional array
- Change the way to make index for brightness
- Make command functions to an array so that it can be called simply
- Change command id for reading device ID
- Change the way to handle the error condition
- Remove power variable, and use the same name variable in bl_dev
- Add the state FB_BLANK_NORMAL to represent the state which the panel
is working but blanked
- Miscellaneous changes to increase the readability and follow the
  coding-style standard

 drivers/gpu/drm/panel/Kconfig|   6 +
 drivers/gpu/drm/panel/Makefile   |   1 +
 drivers/gpu/drm/panel/panel-s6e63j0x03.c | 549 +++
 3 files changed, 556 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-s6e63j0x03.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index bee9f72..bc133e2 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -27,4 +27,10 @@ config DRM_PANEL_S6E8AA0
select DRM_MIPI_DSI
select VIDEOMODE_HELPERS
 
+config DRM_PANEL_S6E63J0X03
+   tristate S6E63J0X03 DSI video mode panel
+   depends on OF
+   select DRM_MIPI_DSI
+   select VIDEOMODE_HELPERS
+
 endmenu
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 8b92921..7f36dc2 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_DRM_PANEL_SIMPLE) += panel-simple.o
 obj-$(CONFIG_DRM_PANEL_LD9040) += panel-ld9040.o
 obj-$(CONFIG_DRM_PANEL_S6E8AA0) += panel-s6e8aa0.o
+obj-$(CONFIG_DRM_PANEL_S6E63J0X03) += panel-s6e63j0x03.o
diff --git a/drivers/gpu/drm/panel/panel-s6e63j0x03.c 
b/drivers/gpu/drm/panel/panel-s6e63j0x03.c
new file mode 100644
index 000..28e4a51
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-s6e63j0x03.c
@@ -0,0 +1,549 @@
+/*
+ * MIPI-DSI based S6E63J0X03 AMOLED lcd 1.63 inch panel driver.
+ *
+ * Copyright (c) 2014 Samsung Electronics Co., Ltd
+ *
+ * Inki Dae, inki@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#include drm/drmP.h
+#include drm/drm_mipi_dsi.h
+#include drm/drm_panel.h
+
+#include linux/of_gpio.h
+#include linux/gpio.h
+#include linux/regulator/consumer.h
+#include linux/backlight.h
+
+#include video/mipi_display.h
+#include video/of_videomode.h
+#include video/videomode.h
+
+#define READ_ID1   0xDA
+#define READ_ID2   0xDB
+#define READ_ID3   0xDC
+
+#define MCS_LEVEL2_KEY 0xf0
+#define MCS_MTP_KEY0xf1
+#define MCS_MTP_SET3   0xd4
+
+#define MIN_BRIGHTNESS 0
+#define MAX_BRIGHTNESS 100
+#define DEFAULT_BRIGHTNESS 80
+
+#define GAMMA_LEVEL_NUM30
+#define NUM_GAMMA_STEPS9
+#define GAMMA_CMD_CNT  28
+
+struct s6e63j0x03 {
+   struct device *dev;
+   struct drm_panel panel;
+   struct backlight_device *bl_dev;
+
+   struct regulator_bulk_data supplies[2];
+   struct gpio_desc *reset_gpio;
+   u32 power_on_delay;
+   u32 power_off_delay;
+   u32 reset_delay;
+   u32 init_delay;
+   bool flip_horizontal;
+   bool flip_vertical;
+   struct videomode vm;
+   unsigned int width_mm;
+   unsigned int height_mm;
+};
+
+static const unsigned char gamma_tbl[NUM_GAMMA_STEPS][GAMMA_CMD_CNT] = {
+   {   /* Gamma 10 */
+   MCS_MTP_SET3,
+   0x00, 0x00, 0x00, 0x7f, 0x7f, 0x7f, 0x52, 0x6b, 0x6f, 0x26,
+   0x28, 0x2d, 0x28, 0x26, 0x27, 0x33, 0x34, 0x32, 0x36, 0x36,
+   0x35, 0x00, 0xab, 0x00, 0xae, 0x00, 0xbf
+   },
+   {   /* gamma 30 */
+   MCS_MTP_SET3,
+   0x00, 0x00, 0x00, 0x70, 0x7f, 0x7f, 0x4e, 0x64, 0x69, 0x26,
+   0x27, 0x2a, 0x28, 0x29, 0x27, 0x31, 0x32, 0x31, 0x35, 0x34,
+   0x35, 0x00, 0xc4, 0x00, 0xca, 0x00, 0xdc
+   },
+   {   /* gamma 60 */
+   MCS_MTP_SET3,
+   0x00, 0x00, 0x00, 0x65, 0x7b, 0x7d, 0x5f, 0x67, 0x68, 0x2a,
+   0x28, 0x29, 0x28, 0x2a, 0x27, 0x31, 0x2f, 0x30, 0x34, 0x33,
+   0x34, 0x00, 0xd9, 0x00, 0xe4, 0x00, 0xf5
+   },
+   {   /* gamma 90 */
+   MCS_MTP_SET3,
+   0x00, 0x00, 0x00, 0x4d, 0x6f, 0x71, 0x67, 0x6a, 0x6c, 0x29,
+   0x28, 0x28, 0x28, 0x29, 0x27, 0x30, 0x2e, 0x30, 0x32