Re: [linux-sunxi] [PATCH 5/9] media: platform: Add Sunxi Cedrus decoder driver
Hi, On Mon, 2018-03-12 at 20:29 +, Joonas Kylmälä wrote: > Paul Kocialkowski: > > diff --git a/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c > > b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c > > new file mode 100644 > > index ..88624035e0e3 > > --- /dev/null > > +++ b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c > > @@ -0,0 +1,313 @@ > > +/* > > + * Sunxi Cedrus codec driver > > + * > > + * Copyright (C) 2016 Florent Revest > > + * Florent Revest > > + * > > + * Based on vim2m > > + * > > + * Copyright (c) 2009-2010 Samsung Electronics Co., Ltd. > > + * Pawel Osciak, > > + * Marek Szyprowski, > > + * > > + * This software is licensed under the terms of the GNU General > > Public > > + * License version 2, as published by the Free Software Foundation, > > and > > + * may be copied, distributed, and modified under those terms. > > + * > > + * This program 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. > > + */ > > + > > +#include "sunxi_cedrus_common.h" > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > I think that the definitions > > #include > #include > #include > #include > #include > #include > > are not used directly in the sunxi_cedrus.c file. Therefore they > should be removed. Thanks for the review, this will be done in v2. Cheers, -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: This is a digitally signed message part
Re: [linux-sunxi] [PATCH 5/9] media: platform: Add Sunxi Cedrus decoder driver
Hi, On Mon, 2018-03-12 at 17:15 +, Joonas Kylmälä wrote: > Paul Kocialkowski: > > diff --git a/drivers/media/platform/sunxi-cedrus/sunxi_cedrus_regs.h > > b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus_regs.h > > new file mode 100644 > > index ..7384daa94737 > > --- /dev/null > > +++ b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus_regs.h > > @@ -0,0 +1,170 @@ > > +/* > > + * Sunxi Cedrus codec driver > > + * > > + * Copyright (C) 2016 Florent Revest > > + * Florent Revest > > + * > > + * Based on Cedrus > > + * > > + * Copyright (c) 2013 Jens Kuske > > + * > > + * This software is licensed under the terms of the GNU General > > Public > > + * License version 2, as published by the Free Software Foundation, > > and > > + * may be copied, distributed, and modified under those terms. > > + * > > + * This program 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. > > + */ > > + > > +#ifndef SUNXI_CEDRUS_REGS_H > > +#define SUNXI_CEDRUS_REGS_H > > + > > +/* > > + * For more information consult http://linux-sunxi.org/VE_Register_ > > guide > > + */ > > + > > +/* Special registers values */ > > + > > +/* VE_CTRL: > > + * The first 3 bits indicate the engine (0 for MPEG, 1 for H264, b > > for AVC...) > > + * The 16th and 17th bits indicate the memory type (3 for DDR3 32 > > bits) > > + * The 20th bit is unknown but needed > > + */ > > +#define VE_CTRL_MPEG 0x13 > > +#define VE_CTRL_H264 0x130001 > > +#define VE_CTRL_AVC0x13000b > > +#define VE_CTRL_REINIT 0x130007 > > + > > +/* VE_MPEG_CTRL: > > + * The bit 3 (0x8) is used to enable IRQs > > + * The other bits are unknown but needed > > + */ > > +#define VE_MPEG_CTRL_MPEG2 0x81b8 > > +#define VE_MPEG_CTRL_MPEG4 (0x80084118 | BIT(7)) > > +#define VE_MPEG_CTRL_MPEG4_P (VE_MPEG_CTRL_MPEG4 | BIT(12)) > > + > > +/* VE_MPEG_VLD_ADDR: > > + * The bits 27 to 4 are used for the address > > + * The bits 31 to 28 (0x7) are used to select the MPEG or JPEG > > engine > > + */ > > +#define VE_MPEG_VLD_ADDR_VAL(x)((x & 0x0ff0) | (x >> > > 28) | (0x7 << 28)) > > + > > +/* VE_MPEG_TRIGGER: > > + * The first three bits are used to trigger the engine > > + * The bits 24 to 26 are used to select the input format (1 for > > MPEG1, 2 for > > Trailing whitespace. Will fix in v2, thanks! Cheers, -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: This is a digitally signed message part
Re: [linux-sunxi] [PATCH 5/9] media: platform: Add Sunxi Cedrus decoder driver
Paul Kocialkowski: > diff --git a/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c > b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c > new file mode 100644 > index ..88624035e0e3 > --- /dev/null > +++ b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus.c > @@ -0,0 +1,313 @@ > +/* > + * Sunxi Cedrus codec driver > + * > + * Copyright (C) 2016 Florent Revest > + * Florent Revest > + * > + * Based on vim2m > + * > + * Copyright (c) 2009-2010 Samsung Electronics Co., Ltd. > + * Pawel Osciak, > + * Marek Szyprowski, > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program 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. > + */ > + > +#include "sunxi_cedrus_common.h" > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include I think that the definitions #include #include #include #include #include #include are not used directly in the sunxi_cedrus.c file. Therefore they should be removed. Joonas
Re: [linux-sunxi] [PATCH 5/9] media: platform: Add Sunxi Cedrus decoder driver
Paul Kocialkowski: > diff --git a/drivers/media/platform/sunxi-cedrus/sunxi_cedrus_regs.h > b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus_regs.h > new file mode 100644 > index ..7384daa94737 > --- /dev/null > +++ b/drivers/media/platform/sunxi-cedrus/sunxi_cedrus_regs.h > @@ -0,0 +1,170 @@ > +/* > + * Sunxi Cedrus codec driver > + * > + * Copyright (C) 2016 Florent Revest > + * Florent Revest > + * > + * Based on Cedrus > + * > + * Copyright (c) 2013 Jens Kuske > + * > + * This software is licensed under the terms of the GNU General Public > + * License version 2, as published by the Free Software Foundation, and > + * may be copied, distributed, and modified under those terms. > + * > + * This program 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. > + */ > + > +#ifndef SUNXI_CEDRUS_REGS_H > +#define SUNXI_CEDRUS_REGS_H > + > +/* > + * For more information consult http://linux-sunxi.org/VE_Register_guide > + */ > + > +/* Special registers values */ > + > +/* VE_CTRL: > + * The first 3 bits indicate the engine (0 for MPEG, 1 for H264, b for > AVC...) > + * The 16th and 17th bits indicate the memory type (3 for DDR3 32 bits) > + * The 20th bit is unknown but needed > + */ > +#define VE_CTRL_MPEG 0x13 > +#define VE_CTRL_H264 0x130001 > +#define VE_CTRL_AVC 0x13000b > +#define VE_CTRL_REINIT 0x130007 > + > +/* VE_MPEG_CTRL: > + * The bit 3 (0x8) is used to enable IRQs > + * The other bits are unknown but needed > + */ > +#define VE_MPEG_CTRL_MPEG2 0x81b8 > +#define VE_MPEG_CTRL_MPEG4 (0x80084118 | BIT(7)) > +#define VE_MPEG_CTRL_MPEG4_P (VE_MPEG_CTRL_MPEG4 | BIT(12)) > + > +/* VE_MPEG_VLD_ADDR: > + * The bits 27 to 4 are used for the address > + * The bits 31 to 28 (0x7) are used to select the MPEG or JPEG engine > + */ > +#define VE_MPEG_VLD_ADDR_VAL(x) ((x & 0x0ff0) | (x >> 28) | (0x7 << > 28)) > + > +/* VE_MPEG_TRIGGER: > + * The first three bits are used to trigger the engine > + * The bits 24 to 26 are used to select the input format (1 for MPEG1, 2 for Trailing whitespace.