On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou [off-list ref] wrote:
quoted hunk
From: Jing Xiang <redacted>
Add pitch length info of graphics/video layer for mmp_win, if it is
YUV format of video layer, u/v pitch will non-zero.
Signed-off-by: Jing Xiang <redacted>
Signed-off-by: Jett.Zhou <redacted>
---
include/video/mmp_disp.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
index b9dd1fb..462e3bd 100644
--- a/include/video/mmp_disp.h
+++ b/include/video/mmp_disp.h
@@ -91,6 +91,11 @@ struct mmp_win {
u16 up_crop;
u16 bottom_crop;
int pix_fmt;
+ /*
+ * pitch[0]: graphics/video layer line length or y pitch
+ * pitch[1]/pitch[2]: video u/v pitch if non-zero
+ */
+ u32 pitch[3];
};
Thanks for adding a comment here, but the meaning of this field is
still not clear to me.
In what case is pitch[0] line length, and in which case does it refer
to y pitch?
pitch[1] and pitch[2] refer to u/v pitch respectively, if their own
values are non-zero? (or if not, what value does the "if non-zero"
comment refer to?)
I would recommend rolling this patch into the patch that actually
makes use of this new field.
Daniel
2013/6/22 Daniel Drake [off-list ref]:
On Mon, Jun 10, 2013 at 9:52 AM, Jett.Zhou [off-list ref] wrote:
quoted
From: Jing Xiang <redacted>
Add pitch length info of graphics/video layer for mmp_win, if it is
YUV format of video layer, u/v pitch will non-zero.
Signed-off-by: Jing Xiang <redacted>
Signed-off-by: Jett.Zhou <redacted>
---
include/video/mmp_disp.h | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/include/video/mmp_disp.h b/include/video/mmp_disp.h
index b9dd1fb..462e3bd 100644
--- a/include/video/mmp_disp.h
+++ b/include/video/mmp_disp.h
@@ -91,6 +91,11 @@ struct mmp_win {
u16 up_crop;
u16 bottom_crop;
int pix_fmt;
+ /*
+ * pitch[0]: graphics/video layer line length or y pitch
+ * pitch[1]/pitch[2]: video u/v pitch if non-zero
+ */
+ u32 pitch[3];
};
Thanks for adding a comment here, but the meaning of this field is
still not clear to me.
In what case is pitch[0] line length, and in which case does it refer
to y pitch?
pitch[1] and pitch[2] refer to u/v pitch respectively, if their own
values are non-zero? (or if not, what value does the "if non-zero"
comment refer to?)
I would recommend rolling this patch into the patch that actually
makes use of this new field.
Daniel
Hi Daniel
pitch is used to represent line length in byte, the usage depends
on pix_fmt.
If the fmt is YUV , the pitch[0] will be Y length, pitch[1] will
be U length, pitch[2] will be V lenth.
If the fmt is RGB, the picth[0] will be line lenth, and
pitch[1]/pitch[2] will be 0 and not be used.
You can refer to pixfmt_to_stride func implementation.
For the patch rolling, do you mean combine the patch5 and patch6
by one patch?
Thanks
--
----------------------------------
Best Regards
Jett Zhou
On Mon, Jun 24, 2013 at 4:34 AM, jett zhou [off-list ref] wrote:
pitch is used to represent line length in byte, the usage depends
on pix_fmt.
If the fmt is YUV , the pitch[0] will be Y length, pitch[1] will
be U length, pitch[2] will be V lenth.
If the fmt is RGB, the picth[0] will be line lenth, and
pitch[1]/pitch[2] will be 0 and not be used.
This description is clear, thanks - hopefully you can write it with
such clarity in the comment :)
For the patch rolling, do you mean combine the patch5 and patch6
by one patch?
I view patch 6 as a cleanup (consolidating and removing duplication of
code), so I would leave that one separate. Patch 6 should not interact
with any pitch[] stuff.
Then you can write a followup patch which adds the pitch[] header,
*and* modifies mmpfb_set_par() to write to pitch[], *and* acts upon
pitch[] in dmafetch_set_fmt (patch 7). This way, the pitch variable is
defined, documented, written to, and acted upon all in the same patch,
the meaning will then be very clear.
Thanks
Daniel
2013/6/25 Daniel Drake [off-list ref]:
On Mon, Jun 24, 2013 at 4:34 AM, jett zhou [off-list ref] wrote:
quoted
pitch is used to represent line length in byte, the usage depends
on pix_fmt.
If the fmt is YUV , the pitch[0] will be Y length, pitch[1] will
be U length, pitch[2] will be V lenth.
If the fmt is RGB, the picth[0] will be line lenth, and
pitch[1]/pitch[2] will be 0 and not be used.
This description is clear, thanks - hopefully you can write it with
such clarity in the comment :)
quoted
For the patch rolling, do you mean combine the patch5 and patch6
by one patch?
I view patch 6 as a cleanup (consolidating and removing duplication of
code), so I would leave that one separate. Patch 6 should not interact
with any pitch[] stuff.
Then you can write a followup patch which adds the pitch[] header,
*and* modifies mmpfb_set_par() to write to pitch[], *and* acts upon
pitch[] in dmafetch_set_fmt (patch 7). This way, the pitch variable is
defined, documented, written to, and acted upon all in the same patch,
the meaning will then be very clear.
HI Daniel
Thanks for your comments.
I will add more detail description on the comments.
For patch6, I will seperated it. For another patch, I will combine
pitch header and mmpfb_set_par and dmafetch_set_fmt (patch 7) as one
new patch based on patch6.
Will send for your review later.
Thanks
--
----------------------------------
Best Regards
Jett Zhou