Hi Ajay,
Thanks for the patch. I have a few comments below.
On Sat, Mar 17, 2012 at 04:52:14PM +0530, Ajay Kumar wrote:
> Modify the G2D driver(which initially supported only FIMG2D v3 style H/W)
> to support FIMG2D v41 style hardware present on Exynos4x12 and Exynos52x0 SOC.
>
> -- Set the SRC and DST type to 'memory' instead of using reset values.
> -- FIMG2D v41 H/W uses different logic for stretching(scaling).
> -- Use CACHECTL_REG only with FIMG2D v3.
>
> Signed-off-by: Ajay Kumar <ajaykumar.rs@xxxxxxxxxxx>
> ---
> drivers/media/video/s5p-g2d/g2d-hw.c | 39 ++++++++++++++++++++++++++++---
> drivers/media/video/s5p-g2d/g2d-regs.h | 6 +++++
> drivers/media/video/s5p-g2d/g2d.c | 23 +++++++++++++++++-
> drivers/media/video/s5p-g2d/g2d.h | 9 ++++++-
> 4 files changed, 70 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/media/video/s5p-g2d/g2d-hw.c b/drivers/media/video/s5p-g2d/g2d-hw.c
> index 5b86cbe..f8225b8 100644
> --- a/drivers/media/video/s5p-g2d/g2d-hw.c
> +++ b/drivers/media/video/s5p-g2d/g2d-hw.c
> @@ -28,6 +28,8 @@ void g2d_set_src_size(struct g2d_dev *d, struct g2d_frame *f)
> {
> u32 n;
>
> + w(0, SRC_SELECT_REG);
> +
> w(f->stride & 0xFFFF, SRC_STRIDE_REG);
>
> n = f->o_height & 0xFFF;
> @@ -52,6 +54,8 @@ void g2d_set_dst_size(struct g2d_dev *d, struct g2d_frame *f)
> {
> u32 n;
>
> + w(0, DST_SELECT_REG);
> +
> w(f->stride & 0xFFFF, DST_STRIDE_REG);
>
> n = f->o_height & 0xFFF;
> @@ -82,10 +86,36 @@ void g2d_set_flip(struct g2d_dev *d, u32 r)
> w(r, SRC_MSK_DIRECT_REG);
> }
>
> -u32 g2d_cmd_stretch(u32 e)
> +/**
> + * g2d_calc_scale_factor - convert scale factor to fixed pint 16
Point, perhaps?
> + * @n: numerator
> + * @d: denominator
> + */
> +static unsigned long g2d_calc_scale_factor(int n, int d)
> +{
> + return (n << 16) / d;
> +}
> +
> +void g2d_set_v41_stretch(struct g2d_dev *d, struct g2d_frame *src,
> + struct g2d_frame *dst)
> {
> - e &= 1;
> - return e << 4;
> + int src_w, src_h, dst_w, dst_h;
Is int intentional --- width and height usually can't be negative.
> + u32 wcfg, hcfg;
> +
> + w(DEFAULT_SCALE_MODE, SRC_SCALE_CTRL_REG);
> +
> + src_w = src->c_width;
> + src_h = src->c_height;
> +
> + dst_w = dst->c_width;
> + dst_h = dst->c_height;
> +
> + /* inversed scaling factor: src is numerator */
> + wcfg = g2d_calc_scale_factor(src_w, dst_w);
> + hcfg = g2d_calc_scale_factor(src_h, dst_h);
I think this would be more simple without that many temporary variables and
an extra function.
> + w(wcfg, SRC_XSCALE_REG);
> + w(hcfg, SRC_YSCALE_REG);
> }
>
> void g2d_set_cmd(struct g2d_dev *d, u32 c)
> @@ -96,7 +126,8 @@ void g2d_set_cmd(struct g2d_dev *d, u32 c)
> void g2d_start(struct g2d_dev *d)
> {
> /* Clear cache */
> - w(0x7, CACHECTL_REG);
> + if (d->device_type == TYPE_G2D_3X)
> + w(0x7, CACHECTL_REG);
> /* Enable interrupt */
> w(1, INTEN_REG);
> /* Start G2D engine */
Kind regards,
--
Sakari Ailus
e-mail: sakari.ailus@xxxxxx jabber/XMPP/Gmail: sailus@xxxxxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
