diff mbox series

[v4,09/11] media: s5p-mfc: Load firmware for each run in MFCv12.

Message ID 20231025102216.50480-10-aakarsh.jain@samsung.com (mailing list archive)
State New, archived
Headers show
Series [v4,01/11] dt-bindings: media: s5p-mfc: Add mfcv12 variant | expand

Commit Message

Aakarsh Jain Oct. 25, 2023, 10:22 a.m. UTC
In MFCv12, some section of firmware gets updated at each MFC run.
Hence we need to reload original firmware for each run at the start.

Cc: linux-fsd@tesla.com
Signed-off-by: Smitha T Murthy <smithatmurthy@gmail.com>
Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
---
 drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Hans Verkuil Nov. 22, 2023, 3:44 p.m. UTC | #1
On 25/10/2023 12:22, Aakarsh Jain wrote:
> In MFCv12, some section of firmware gets updated at each MFC run.
> Hence we need to reload original firmware for each run at the start.

Huh? This is very weird. This definitely deserves a comment in the actual
code rather than just the commit log.

Do you know what is going on? What part is updated? Are you sure it isn't
a driver bug somehow?

Regards,

	Hans

> 
> Cc: linux-fsd@tesla.com
> Signed-off-by: Smitha T Murthy <smithatmurthy@gmail.com>
> Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> ---
>  drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> index b49159142c53..057088b9d327 100644
> --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> @@ -51,8 +51,9 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
>  	 * into kernel. */
>  	mfc_debug_enter();
>  
> -	if (dev->fw_get_done)
> -		return 0;
> +	if (!IS_MFCV12(dev))
> +		if (dev->fw_get_done)
> +			return 0;
>  
>  	for (i = MFC_FW_MAX_VERSIONS - 1; i >= 0; i--) {
>  		if (!dev->variant->fw_name[i])
Aakarsh Jain Nov. 28, 2023, 10:31 a.m. UTC | #2
Hi Hans,

> -----Original Message-----
> From: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Sent: 22 November 2023 21:14
> To: Aakarsh Jain <aakarsh.jain@samsung.com>; linux-arm-
> kernel@lists.infradead.org; linux-media@vger.kernel.org; linux-
> kernel@vger.kernel.org; devicetree@vger.kernel.org
> Cc: m.szyprowski@samsung.com; andrzej.hajda@intel.com;
> mchehab@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> dillon.minfei@gmail.com; david.plowman@raspberrypi.com;
> mark.rutland@arm.com; robh+dt@kernel.org; conor+dt@kernel.org; linux-
> samsung-soc@vger.kernel.org; andi@etezian.org; gost.dev@samsung.com;
> alim.akhtar@samsung.com; aswani.reddy@samsung.com;
> pankaj.dubey@samsung.com; ajaykumar.rs@samsung.com; linux-
> fsd@tesla.com; Smitha T Murthy <smithatmurthy@gmail.com>
> Subject: Re: [Patch v4 09/11] media: s5p-mfc: Load firmware for each run in
> MFCv12.
> 
> On 25/10/2023 12:22, Aakarsh Jain wrote:
> > In MFCv12, some section of firmware gets updated at each MFC run.
> > Hence we need to reload original firmware for each run at the start.
> 
> Huh? This is very weird. This definitely deserves a comment in the actual
> code rather than just the commit log.
> 
> Do you know what is going on? What part is updated? Are you sure it isn't a
> driver bug somehow?
> 
> Regards,
> 
> 	Hans
> 
During SYS_INIT command sent to MFC sequentially, firmware is not able to initialize the hardware due to incorrect firmware transfer and in current scenario the firmware is not loaded again in the Reserved memory area.
In this case RET_SYS_INIT response from hardware is failing. So we need to load firmware every time we open the device node.
I will add comment in the code why this change is needed.

Thanks for the review.
> >
> > Cc: linux-fsd@tesla.com
> > Signed-off-by: Smitha T Murthy <smithatmurthy@gmail.com>
> > Signed-off-by: Aakarsh Jain <aakarsh.jain@samsung.com>
> > ---
> >  drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > index b49159142c53..057088b9d327 100644
> > --- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > +++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
> > @@ -51,8 +51,9 @@ int s5p_mfc_load_firmware(struct s5p_mfc_dev
> *dev)
> >  	 * into kernel. */
> >  	mfc_debug_enter();
> >
> > -	if (dev->fw_get_done)
> > -		return 0;
> > +	if (!IS_MFCV12(dev))
> > +		if (dev->fw_get_done)
> > +			return 0;
> >
> >  	for (i = MFC_FW_MAX_VERSIONS - 1; i >= 0; i--) {
> >  		if (!dev->variant->fw_name[i])
diff mbox series

Patch

diff --git a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
index b49159142c53..057088b9d327 100644
--- a/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
+++ b/drivers/media/platform/samsung/s5p-mfc/s5p_mfc_ctrl.c
@@ -51,8 +51,9 @@  int s5p_mfc_load_firmware(struct s5p_mfc_dev *dev)
 	 * into kernel. */
 	mfc_debug_enter();
 
-	if (dev->fw_get_done)
-		return 0;
+	if (!IS_MFCV12(dev))
+		if (dev->fw_get_done)
+			return 0;
 
 	for (i = MFC_FW_MAX_VERSIONS - 1; i >= 0; i--) {
 		if (!dev->variant->fw_name[i])