| Message ID | 1394181090-16446-1-git-send-email-arun.kk@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On 07/03/14 09:31, Arun Kumar K wrote: > From: avnd kiran <avnd.kiran@samsung.com> > > Latest MFC v6 firmware requires tile mode and loop filter > setting to be done as part of Init buffer command, in sync > with v7. So, move these settings out of decode options reg. > Also, make this register definition applicable from v6 onwards. > > Signed-off-by: avnd kiran <avnd.kiran@samsung.com> > Signed-off-by: Arun Kumar K <arun.kk@samsung.com> Will the driver also work with older version of the firmware after this change ? If not, shouldn't things like this be done depending on what firmware version is loaded ? Regards, Sylwester -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Sylwester, On Fri, Mar 7, 2014 at 2:59 PM, Sylwester Nawrocki <s.nawrocki@samsung.com> wrote: > Hi, > > On 07/03/14 09:31, Arun Kumar K wrote: >> From: avnd kiran <avnd.kiran@samsung.com> >> >> Latest MFC v6 firmware requires tile mode and loop filter >> setting to be done as part of Init buffer command, in sync >> with v7. So, move these settings out of decode options reg. >> Also, make this register definition applicable from v6 onwards. >> >> Signed-off-by: avnd kiran <avnd.kiran@samsung.com> >> Signed-off-by: Arun Kumar K <arun.kk@samsung.com> > > Will the driver also work with older version of the firmware > after this change ? If not, shouldn't things like this be done > depending on what firmware version is loaded ? > The original code was for the initial version of v6 firmware. After that the v6 firmware has got many fixes and updates which also got updated in the products running the same. As such there are no official multiple versions of v6 firmware, but only fixes / updates to older version. I will update the s5p-mfc-v6.fw in the linux-firmware also with the newer version. Hope that will be fine. Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Arun, > From: Arun Kumar K [mailto:arunkk.samsung@gmail.com] > Sent: Friday, March 07, 2014 12:10 PM > > Hi Sylwester, > > On Fri, Mar 7, 2014 at 2:59 PM, Sylwester Nawrocki > <s.nawrocki@samsung.com> wrote: > > Hi, > > > > On 07/03/14 09:31, Arun Kumar K wrote: > >> From: avnd kiran <avnd.kiran@samsung.com> > >> > >> Latest MFC v6 firmware requires tile mode and loop filter setting to > >> be done as part of Init buffer command, in sync with v7. So, move > >> these settings out of decode options reg. > >> Also, make this register definition applicable from v6 onwards. > >> > >> Signed-off-by: avnd kiran <avnd.kiran@samsung.com> > >> Signed-off-by: Arun Kumar K <arun.kk@samsung.com> > > > > Will the driver also work with older version of the firmware after > > this change ? If not, shouldn't things like this be done depending on > > what firmware version is loaded ? > > > > The original code was for the initial version of v6 firmware. > After that the v6 firmware has got many fixes and updates which also > got updated in the products running the same. > As such there are no official multiple versions of v6 firmware, but > only fixes / updates to older version. I will update the s5p-mfc-v6.fw > in the linux-firmware also with the newer version. Hope that will be > fine. Unfortunately, I share the same concerns as Sylwester. We have two problems: 1) new kernel + old firmware In this case, someone will update the kernel and find out that video decoding is not working. An assumption that I think is common, is that updating the kernel should not break anything. If it was working with the previous version it should work with the next. The solution I can suggest is that a check which firmware version is used has to be implemented. Maybe you can use the date of firmware to do this check? 2) old kernel + new firmware I see no clear solution to this problem. If the kernel is old and the firmware is behaving differently, the video decoding will not work. I can guess that this case would be less common, but still a person can update the firmware and leave the old kernel. Changing the firmware can be done by replacing a single file. In addition to the above, you need to clearly specify in the linux-firmware.git what is going on. A readme file is a must. Maybe a second v6 firmware file should be included? Best wishes,
Hi Kamil, On Fri, Mar 7, 2014 at 6:18 PM, Kamil Debski <k.debski@samsung.com> wrote: > Hi Arun, > >> From: Arun Kumar K [mailto:arunkk.samsung@gmail.com] >> Sent: Friday, March 07, 2014 12:10 PM >> >> Hi Sylwester, >> >> On Fri, Mar 7, 2014 at 2:59 PM, Sylwester Nawrocki >> <s.nawrocki@samsung.com> wrote: >> > Hi, >> > >> > On 07/03/14 09:31, Arun Kumar K wrote: >> >> From: avnd kiran <avnd.kiran@samsung.com> >> >> >> >> Latest MFC v6 firmware requires tile mode and loop filter setting to >> >> be done as part of Init buffer command, in sync with v7. So, move >> >> these settings out of decode options reg. >> >> Also, make this register definition applicable from v6 onwards. >> >> >> >> Signed-off-by: avnd kiran <avnd.kiran@samsung.com> >> >> Signed-off-by: Arun Kumar K <arun.kk@samsung.com> >> > >> > Will the driver also work with older version of the firmware after >> > this change ? If not, shouldn't things like this be done depending on >> > what firmware version is loaded ? >> > >> >> The original code was for the initial version of v6 firmware. >> After that the v6 firmware has got many fixes and updates which also >> got updated in the products running the same. >> As such there are no official multiple versions of v6 firmware, but >> only fixes / updates to older version. I will update the s5p-mfc-v6.fw >> in the linux-firmware also with the newer version. Hope that will be >> fine. > > Unfortunately, I share the same concerns as Sylwester. We have two problems: > 1) new kernel + old firmware > > In this case, someone will update the kernel and find out that video > decoding is not working. An assumption that I think is common, is that > updating the kernel should not break anything. If it was working with the > previous version it should work with the next. > > The solution I can suggest is that a check which firmware version is used > has to be implemented. Maybe you can use the date of firmware to do this > check? > Yes this concern is valid. I think its better to check for firmware date as the old firmware is already submitted in mainline. > 2) old kernel + new firmware > > I see no clear solution to this problem. If the kernel is old and the > firmware is behaving differently, the video decoding will not work. I can > guess that this case would be less common, but still a person can update the > firmware and leave the old kernel. Changing the firmware can be done by > replacing a single file. > > In addition to the above, you need to clearly specify in the > linux-firmware.git what is going on. A readme file is a must. Maybe a second > v6 firmware file should be included? > Yes I can put the newer version of v6 firmware also in linux-firmware with a readme detailing the difference between the same. I hope this is a valid solution. Regards Arun -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h index 8d0b686..b47567c 100644 --- a/drivers/media/platform/s5p-mfc/regs-mfc-v6.h +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v6.h @@ -132,6 +132,7 @@ #define S5P_FIMV_D_METADATA_BUFFER_ADDR_V6 0xf448 #define S5P_FIMV_D_METADATA_BUFFER_SIZE_V6 0xf44c #define S5P_FIMV_D_NUM_MV_V6 0xf478 +#define S5P_FIMV_D_INIT_BUFFER_OPTIONS_V6 0xf47c #define S5P_FIMV_D_CPB_BUFFER_ADDR_V6 0xf4b0 #define S5P_FIMV_D_CPB_BUFFER_SIZE_V6 0xf4b4 diff --git a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h index ea5ec2a..82c96fa 100644 --- a/drivers/media/platform/s5p-mfc/regs-mfc-v7.h +++ b/drivers/media/platform/s5p-mfc/regs-mfc-v7.h @@ -18,8 +18,6 @@ #define S5P_FIMV_CODEC_VP8_ENC_V7 25 /* Additional registers for v7 */ -#define S5P_FIMV_D_INIT_BUFFER_OPTIONS_V7 0xf47c - #define S5P_FIMV_E_SOURCE_FIRST_ADDR_V7 0xf9e0 #define S5P_FIMV_E_SOURCE_SECOND_ADDR_V7 0xf9e4 #define S5P_FIMV_E_SOURCE_THIRD_ADDR_V7 0xf9e8 diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c index 90edb19..b226d75 100644 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_opr_v6.c @@ -1296,10 +1296,8 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx) WRITEL(ctx->display_delay, S5P_FIMV_D_DISPLAY_DELAY_V6); } - if (IS_MFCV7(dev)) { - WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6); - reg = 0; - } + WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6); + reg = 0; /* Setup loop filter, for decoding this is only valid for MPEG4 */ if (ctx->codec_mode == S5P_MFC_CODEC_MPEG4_DEC) { @@ -1311,10 +1309,7 @@ static int s5p_mfc_init_decode_v6(struct s5p_mfc_ctx *ctx) if (ctx->dst_fmt->fourcc == V4L2_PIX_FMT_NV12MT_16X16) reg |= (0x1 << S5P_FIMV_D_OPT_TILE_MODE_SHIFT_V6); - if (IS_MFCV7(dev)) - WRITEL(reg, S5P_FIMV_D_INIT_BUFFER_OPTIONS_V7); - else - WRITEL(reg, S5P_FIMV_D_DEC_OPTIONS_V6); + WRITEL(reg, S5P_FIMV_D_INIT_BUFFER_OPTIONS_V6); /* 0: NV12(CbCr), 1: NV21(CrCb) */ if (ctx->dst_fmt->fourcc == V4L2_PIX_FMT_NV21M)