Re: [PATCH 2/3] [media] s5p-mfc: Core support to add v8 decoder

[Arun Kumar K <arunkk.samsung@gmail.com>]

Hi Sachin,

Thank you for the review.

On 04/29/14 22:45, Sachin Kamat wrote:
> Hi Arun,
>
> On 23 April 2014 18:27, Arun Kumar K <arun.kk@samsung.com> wrote:
>> From: Kiran AVND <avnd.kiran@samsung.com>
>>
>> This patch adds variant data and core support for
>> V8 decoder. This patch also adds the register definition
>> file for new firmware version v8 for MFC.
>>
>> Signed-off-by: Kiran AVND <avnd.kiran@samsung.com>
>> Signed-off-by: Pawel Osciak <posciak@chromium.org>
>> Signed-off-by: Arun Kumar K <arun.kk@samsung.com>
>> ---
> <...>
>> +
>> +/* Returned value register for specific setting */
>> +#define S5P_FIMV_D_RET_PICTURE_TAG_TOP_V8      0xf674
>> +#define S5P_FIMV_D_RET_PICTURE_TAG_BOT_V8      0xf678
>> +#define S5P_FIMV_D_MVC_VIEW_ID_V8              0xf6d8
>> +
>> +/* SEI related information */
>> +#define S5P_FIMV_D_FRAME_PACK_SEI_AVAIL_V8     0xf6dc
>> +
>> +/* MFCv8 Context buffer sizes */
>> +#define MFC_CTX_BUF_SIZE_V8            (30 * SZ_1K)    /*  30KB */
>
> Please include header file for size macros.
>

The file linux/sizes.h is included in regs-mfc-v6.h which
inturn gets included in this file. Isnt that fine?

> <...>
>>   };
>> diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
>> index 48a14b5..f0e63f5 100644
>> --- a/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
>> +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_common.h
>> @@ -23,8 +23,7 @@
>>   #include <media/v4l2-ioctl.h>
>>   #include <media/videobuf2-core.h>
>>   #include "regs-mfc.h"
>> -#include "regs-mfc-v6.h"
>> -#include "regs-mfc-v7.h"
>> +#include "regs-mfc-v8.h"
>>
>>   /* Definitions related to MFC memory */
>>
>> @@ -705,5 +704,6 @@ void set_work_bit_irqsave(struct s5p_mfc_ctx *ctx);
>>   #define IS_TWOPORT(dev)                (dev->variant->port_num == 2 ? 1 : 0)
>>   #define IS_MFCV6_PLUS(dev)     (dev->variant->version >= 0x60 ? 1 : 0)
>>   #define IS_MFCV7(dev)          (dev->variant->version >= 0x70 ? 1 : 0)
>
> Is MFC v8 superset of MFC v7?
>

Yes it is a superset.
So the last patch in this series renames IS_MFCV7 to IS_MFCV7_PLUS.

Regards
Arun

>> +#define IS_MFCV8(dev)          (dev->variant->version >= 0x80 ? 1 : 0)
>