diff mbox series

[02/17] media: atomisp: pci: fix punit_ddr_dvfs_enable() argument for mrfld_power up case

Message ID 20211017161958.44351-3-kitakar@gmail.com (mailing list archive)
State Accepted, archived
Headers
Series various fixes for atomisp to make it work |

Commit Message

Tsuchiya Yuto Oct. 17, 2021, 4:19 p.m. UTC 
  When comparing with intel-aero atomisp [1], it looks like
punit_ddr_dvfs_enable() should take `false` as an argument on mrfld_power
up case.

Code from the intel-aero kernel [1]:

        int atomisp_mrfld_power_down(struct atomisp_device *isp)
        {
        [...]
        	/*WA:Enable DVFS*/
        	if (IS_CHT)
        		punit_ddr_dvfs_enable(true);

        int atomisp_mrfld_power_up(struct atomisp_device *isp)
        {
        [...]
        	/*WA for PUNIT, if DVFS enabled, ISP timeout observed*/
        	if (IS_CHT)
        		punit_ddr_dvfs_enable(false);

This patch fixes the inverted argument as per the intel-aero code, as
well as its comment. While here, fix space issues for comments in
atomisp_mrfld_power().

Note that it does not seem to be possible to unify the up/down cases for
punit_ddr_dvfs_enable(), i.e., we can't do something like the following:

        if (IS_CHT)
        	punit_ddr_dvfs_enable(!enable);

because according to the intel-aero code [1], the DVFS is disabled
before "writing 0x0 to ISPSSPM0 bit[1:0]" and the DVFS is enabled after
"writing 0x3 to ISPSSPM0 bit[1:0]".

[1] https://github.com/intel-aero/linux-kernel/blob/a1b673258feb915268377275130c5c5df0eafc82/drivers/media/pci/atomisp/atomisp_driver/atomisp_v4l2.c#L431-L514

Fixes: 0f441fd70b1e ("media: atomisp: simplify the power down/up code")
Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
---
 drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Andy Shevchenko Oct. 18, 2021, 11:07 a.m. UTC | #1 
On Mon, Oct 18, 2021 at 01:19:42AM +0900, Tsuchiya Yuto wrote:
> When comparing with intel-aero atomisp [1], it looks like
> punit_ddr_dvfs_enable() should take `false` as an argument on mrfld_power
> up case.
> 
> Code from the intel-aero kernel [1]:
> 
>         int atomisp_mrfld_power_down(struct atomisp_device *isp)
>         {
>         [...]
>         	/*WA:Enable DVFS*/
>         	if (IS_CHT)
>         		punit_ddr_dvfs_enable(true);
> 
>         int atomisp_mrfld_power_up(struct atomisp_device *isp)
>         {
>         [...]
>         	/*WA for PUNIT, if DVFS enabled, ISP timeout observed*/
>         	if (IS_CHT)
>         		punit_ddr_dvfs_enable(false);
> 
> This patch fixes the inverted argument as per the intel-aero code, as
> well as its comment. While here, fix space issues for comments in
> atomisp_mrfld_power().
> 
> Note that it does not seem to be possible to unify the up/down cases for
> punit_ddr_dvfs_enable(), i.e., we can't do something like the following:
> 
>         if (IS_CHT)
>         	punit_ddr_dvfs_enable(!enable);
> 
> because according to the intel-aero code [1], the DVFS is disabled
> before "writing 0x0 to ISPSSPM0 bit[1:0]" and the DVFS is enabled after
> "writing 0x3 to ISPSSPM0 bit[1:0]".
> 
> [1] https://github.com/intel-aero/linux-kernel/blob/a1b673258feb915268377275130c5c5df0eafc82/drivers/media/pci/atomisp/atomisp_driver/atomisp_v4l2.c#L431-L514
> 
> Fixes: 0f441fd70b1e ("media: atomisp: simplify the power down/up code")
> Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
> ---
>  drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 0511c454e769..f5362554638e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -711,15 +711,15 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
>  
>  	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
>  
> -	/*WA:Enable DVFS*/
> +	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */

P-Unit

>  	if (IS_CHT && enable)
> -		punit_ddr_dvfs_enable(true);
> +		punit_ddr_dvfs_enable(false);
>  
>  	/*
>  	 * FIXME:WA for ECS28A, with this sleep, CTS
>  	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
>  	 * PASS, no impact on other platforms
> -	*/
> +	 */
>  	if (IS_BYT && enable)
>  		msleep(10);
>  
> @@ -727,7 +727,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
>  	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
>  			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
>  
> -	/*WA:Enable DVFS*/
> +	/* WA:Enable DVFS */
>  	if (IS_CHT && !enable)
>  		punit_ddr_dvfs_enable(true);
>  
> -- 
> 2.33.1
> 
>
Tsuchiya Yuto Oct. 20, 2021, 1:25 p.m. UTC | #2 
On Mon, 2021-10-18 at 14:07 +0300, Andy Shevchenko wrote:
> On Mon, Oct 18, 2021 at 01:19:42AM +0900, Tsuchiya Yuto wrote:
> > When comparing with intel-aero atomisp [1], it looks like
> > punit_ddr_dvfs_enable() should take `false` as an argument on mrfld_power
> > up case.
> > 
> > Code from the intel-aero kernel [1]:
> > 
> >         int atomisp_mrfld_power_down(struct atomisp_device *isp)
> >         {
> >         [...]
> >         	/*WA:Enable DVFS*/
> >         	if (IS_CHT)
> >         		punit_ddr_dvfs_enable(true);
> > 
> >         int atomisp_mrfld_power_up(struct atomisp_device *isp)
> >         {
> >         [...]
> >         	/*WA for PUNIT, if DVFS enabled, ISP timeout observed*/
> >         	if (IS_CHT)
> >         		punit_ddr_dvfs_enable(false);
> > 
> > This patch fixes the inverted argument as per the intel-aero code, as
> > well as its comment. While here, fix space issues for comments in
> > atomisp_mrfld_power().
> > 
> > Note that it does not seem to be possible to unify the up/down cases for
> > punit_ddr_dvfs_enable(), i.e., we can't do something like the following:
> > 
> >         if (IS_CHT)
> >         	punit_ddr_dvfs_enable(!enable);
> > 
> > because according to the intel-aero code [1], the DVFS is disabled
> > before "writing 0x0 to ISPSSPM0 bit[1:0]" and the DVFS is enabled after
> > "writing 0x3 to ISPSSPM0 bit[1:0]".
> > 
> > [1] https://github.com/intel-aero/linux-kernel/blob/a1b673258feb915268377275130c5c5df0eafc82/drivers/media/pci/atomisp/atomisp_driver/atomisp_v4l2.c#L431-L514
> > 
> > Fixes: 0f441fd70b1e ("media: atomisp: simplify the power down/up code")
> > Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
> > ---
> >  drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > index 0511c454e769..f5362554638e 100644
> > --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > @@ -711,15 +711,15 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> >  
> >  	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
> >  
> > -	/*WA:Enable DVFS*/
> > +	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */
> 
> P-Unit

Thanks, I'll fix this next time I send.

> >  	if (IS_CHT && enable)
> > -		punit_ddr_dvfs_enable(true);
> > +		punit_ddr_dvfs_enable(false);
> >  
> >  	/*
> >  	 * FIXME:WA for ECS28A, with this sleep, CTS
> >  	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
> >  	 * PASS, no impact on other platforms
> > -	*/
> > +	 */
> >  	if (IS_BYT && enable)
> >  		msleep(10);
> >  
> > @@ -727,7 +727,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> >  	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
> >  			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
> >  
> > -	/*WA:Enable DVFS*/
> > +	/* WA:Enable DVFS */
> >  	if (IS_CHT && !enable)
> >  		punit_ddr_dvfs_enable(true);
> >  
> > -- 
> > 2.33.1
> > 
> > 
>
Dan Carpenter Nov. 2, 2021, 11:26 a.m. UTC | #3 
On Mon, Oct 18, 2021 at 01:19:42AM +0900, Tsuchiya Yuto wrote:
> When comparing with intel-aero atomisp [1], it looks like
> punit_ddr_dvfs_enable() should take `false` as an argument on mrfld_power
> up case.
> 
> Code from the intel-aero kernel [1]:
> 
>         int atomisp_mrfld_power_down(struct atomisp_device *isp)
>         {
>         [...]
>         	/*WA:Enable DVFS*/
>         	if (IS_CHT)
>         		punit_ddr_dvfs_enable(true);
> 
>         int atomisp_mrfld_power_up(struct atomisp_device *isp)
>         {
>         [...]
>         	/*WA for PUNIT, if DVFS enabled, ISP timeout observed*/
>         	if (IS_CHT)
>         		punit_ddr_dvfs_enable(false);
> 
> This patch fixes the inverted argument as per the intel-aero code, as
> well as its comment. While here, fix space issues for comments in
> atomisp_mrfld_power().
> 
> Note that it does not seem to be possible to unify the up/down cases for
> punit_ddr_dvfs_enable(), i.e., we can't do something like the following:
> 
>         if (IS_CHT)
>         	punit_ddr_dvfs_enable(!enable);
> 
> because according to the intel-aero code [1], the DVFS is disabled
> before "writing 0x0 to ISPSSPM0 bit[1:0]" and the DVFS is enabled after
> "writing 0x3 to ISPSSPM0 bit[1:0]".
> 
> [1] https://github.com/intel-aero/linux-kernel/blob/a1b673258feb915268377275130c5c5df0eafc82/drivers/media/pci/atomisp/atomisp_driver/atomisp_v4l2.c#L431-L514
> 
> Fixes: 0f441fd70b1e ("media: atomisp: simplify the power down/up code")
> Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
> ---
>  drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> index 0511c454e769..f5362554638e 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> @@ -711,15 +711,15 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
>  
>  	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
>  
> -	/*WA:Enable DVFS*/
> +	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */
>  	if (IS_CHT && enable)
> -		punit_ddr_dvfs_enable(true);
> +		punit_ddr_dvfs_enable(false);
>  
>  	/*
>  	 * FIXME:WA for ECS28A, with this sleep, CTS
>  	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
>  	 * PASS, no impact on other platforms
> -	*/
> +	 */
        ^^^

>  	if (IS_BYT && enable)
>  		msleep(10);
>  
> @@ -727,7 +727,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
>  	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
>  			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
>  
> -	/*WA:Enable DVFS*/
> +	/* WA:Enable DVFS */
        ^^^^^^^^^^^^^^^^^^^
These two white space changes are unrelated.  Please do them in a
separate patch.

regards,
dan carpenter
Tsuchiya Yuto Nov. 8, 2021, 2:48 p.m. UTC | #4 
On Tue, 2021-11-02 at 14:26 +0300, Dan Carpenter wrote:
> On Mon, Oct 18, 2021 at 01:19:42AM +0900, Tsuchiya Yuto wrote:
> > When comparing with intel-aero atomisp [1], it looks like
> > punit_ddr_dvfs_enable() should take `false` as an argument on mrfld_power
> > up case.
> > 
> > Code from the intel-aero kernel [1]:
> > 
> >         int atomisp_mrfld_power_down(struct atomisp_device *isp)
> >         {
> >         [...]
> >         	/*WA:Enable DVFS*/
> >         	if (IS_CHT)
> >         		punit_ddr_dvfs_enable(true);
> > 
> >         int atomisp_mrfld_power_up(struct atomisp_device *isp)
> >         {
> >         [...]
> >         	/*WA for PUNIT, if DVFS enabled, ISP timeout observed*/
> >         	if (IS_CHT)
> >         		punit_ddr_dvfs_enable(false);
> > 
> > This patch fixes the inverted argument as per the intel-aero code, as
> > well as its comment. While here, fix space issues for comments in
> > atomisp_mrfld_power().
> > 
> > Note that it does not seem to be possible to unify the up/down cases for
> > punit_ddr_dvfs_enable(), i.e., we can't do something like the following:
> > 
> >         if (IS_CHT)
> >         	punit_ddr_dvfs_enable(!enable);
> > 
> > because according to the intel-aero code [1], the DVFS is disabled
> > before "writing 0x0 to ISPSSPM0 bit[1:0]" and the DVFS is enabled after
> > "writing 0x3 to ISPSSPM0 bit[1:0]".
> > 
> > [1] https://github.com/intel-aero/linux-kernel/blob/a1b673258feb915268377275130c5c5df0eafc82/drivers/media/pci/atomisp/atomisp_driver/atomisp_v4l2.c#L431-L514
> > 
> > Fixes: 0f441fd70b1e ("media: atomisp: simplify the power down/up code")
> > Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
> > ---
> >  drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > index 0511c454e769..f5362554638e 100644
> > --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > @@ -711,15 +711,15 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> >  
> >  	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
> >  
> > -	/*WA:Enable DVFS*/
> > +	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */
> >  	if (IS_CHT && enable)
> > -		punit_ddr_dvfs_enable(true);
> > +		punit_ddr_dvfs_enable(false);
> >  
> >  	/*
> >  	 * FIXME:WA for ECS28A, with this sleep, CTS
> >  	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
> >  	 * PASS, no impact on other platforms
> > -	*/
> > +	 */
>         ^^^
> 
> >  	if (IS_BYT && enable)
> >  		msleep(10);
> >  
> > @@ -727,7 +727,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> >  	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
> >  			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
> >  
> > -	/*WA:Enable DVFS*/
> > +	/* WA:Enable DVFS */
>         ^^^^^^^^^^^^^^^^^^^
> These two white space changes are unrelated.  Please do them in a
> separate patch.

Thank you for the review :-)

I thought these space fixes were too trivial for a separate patch, so
I made the fix while at it. I have no strong reason not to separate the
space fix. I'll do so in v2.

Regards,
Tsuchiya Yuto
Dan Carpenter Nov. 8, 2021, 3:10 p.m. UTC | #5 
On Mon, Nov 08, 2021 at 11:48:42PM +0900, Tsuchiya Yuto wrote:
> > > diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > index 0511c454e769..f5362554638e 100644
> > > --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > @@ -711,15 +711,15 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> > >  
> > >  	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
> > >  
> > > -	/*WA:Enable DVFS*/
> > > +	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */
> > >  	if (IS_CHT && enable)
> > > -		punit_ddr_dvfs_enable(true);
> > > +		punit_ddr_dvfs_enable(false);
> > >  
> > >  	/*
> > >  	 * FIXME:WA for ECS28A, with this sleep, CTS
> > >  	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
> > >  	 * PASS, no impact on other platforms
> > > -	*/
> > > +	 */
> >         ^^^
> > 
> > >  	if (IS_BYT && enable)
> > >  		msleep(10);
> > >  
> > > @@ -727,7 +727,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> > >  	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
> > >  			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
> > >  
> > > -	/*WA:Enable DVFS*/
> > > +	/* WA:Enable DVFS */
> >         ^^^^^^^^^^^^^^^^^^^
> > These two white space changes are unrelated.  Please do them in a
> > separate patch.
> 
> Thank you for the review :-)
> 
> I thought these space fixes were too trivial for a separate patch, so
> I made the fix while at it. I have no strong reason not to separate the
> space fix. I'll do so in v2.

Trivial white space patches are fine.  We get thousands of those.

This goes through Mauro's tree instead of Greg's and he is definitely
more flexible than Greg.  Plus the atomisp stuff is broken so no one
cares.

But the rules are really clear and it does help in reviewing when people
follow them.

When I'm reviewing this patch, the patch description says it is a fix so
I review that differently.  The fix is very simple and it changes true
to false.

The other patch would have been a "change comments" patch.  We get
thousands of those as I mentioned.  My concern there is that a UMN
researcher will try to trick us by hiding code in a "change comments"
patch so I have a script to review those.  It takes one second to run.

But then when I was reviewing this patch I first had to spot the change
from true to false before I could review it.  That's where most of the
time was wasted.

regards,
dan carpenter
Tsuchiya Yuto Dec. 1, 2021, 12:07 p.m. UTC | #6 
On Wed, 2021-10-20 at 22:25 +0900, Tsuchiya Yuto wrote:
> On Mon, 2021-10-18 at 14:07 +0300, Andy Shevchenko wrote:
> > On Mon, Oct 18, 2021 at 01:19:42AM +0900, Tsuchiya Yuto wrote:
> > > When comparing with intel-aero atomisp [1], it looks like
> > > punit_ddr_dvfs_enable() should take `false` as an argument on mrfld_power
> > > up case.
> > > 
> > > Code from the intel-aero kernel [1]:
> > > 
> > >         int atomisp_mrfld_power_down(struct atomisp_device *isp)
> > >         {
> > >         [...]
> > >         	/*WA:Enable DVFS*/
> > >         	if (IS_CHT)
> > >         		punit_ddr_dvfs_enable(true);
> > > 
> > >         int atomisp_mrfld_power_up(struct atomisp_device *isp)
> > >         {
> > >         [...]
> > >         	/*WA for PUNIT, if DVFS enabled, ISP timeout observed*/
> > >         	if (IS_CHT)
> > >         		punit_ddr_dvfs_enable(false);
> > > 
> > > This patch fixes the inverted argument as per the intel-aero code, as
> > > well as its comment. While here, fix space issues for comments in
> > > atomisp_mrfld_power().
> > > 
> > > Note that it does not seem to be possible to unify the up/down cases for
> > > punit_ddr_dvfs_enable(), i.e., we can't do something like the following:
> > > 
> > >         if (IS_CHT)
> > >         	punit_ddr_dvfs_enable(!enable);
> > > 
> > > because according to the intel-aero code [1], the DVFS is disabled
> > > before "writing 0x0 to ISPSSPM0 bit[1:0]" and the DVFS is enabled after
> > > "writing 0x3 to ISPSSPM0 bit[1:0]".
> > > 
> > > [1] https://github.com/intel-aero/linux-kernel/blob/a1b673258feb915268377275130c5c5df0eafc82/drivers/media/pci/atomisp/atomisp_driver/atomisp_v4l2.c#L431-L514
> > > 
> > > Fixes: 0f441fd70b1e ("media: atomisp: simplify the power down/up code")
> > > Signed-off-by: Tsuchiya Yuto <kitakar@gmail.com>
> > > ---
> > >  drivers/staging/media/atomisp/pci/atomisp_v4l2.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > index 0511c454e769..f5362554638e 100644
> > > --- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > +++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
> > > @@ -711,15 +711,15 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> > >  
> > >  	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
> > >  
> > > -	/*WA:Enable DVFS*/
> > > +	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */
> > 
> > P-Unit
> 
> Thanks, I'll fix this next time I send.

For the record, this is already fixed in the latest media_stage tree.

> > >  	if (IS_CHT && enable)
> > > -		punit_ddr_dvfs_enable(true);
> > > +		punit_ddr_dvfs_enable(false);
> > >  
> > >  	/*
> > >  	 * FIXME:WA for ECS28A, with this sleep, CTS
> > >  	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
> > >  	 * PASS, no impact on other platforms
> > > -	*/
> > > +	 */
> > >  	if (IS_BYT && enable)
> > >  		msleep(10);
> > >  
> > > @@ -727,7 +727,7 @@ static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
> > >  	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
> > >  			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
> > >  
> > > -	/*WA:Enable DVFS*/
> > > +	/* WA:Enable DVFS */
> > >  	if (IS_CHT && !enable)
> > >  		punit_ddr_dvfs_enable(true);
> > >  
> > > -- 
> > > 2.33.1
> > > 
> > > 
> > 
>
diff mbox series

Patch

diff --git a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
index 0511c454e769..f5362554638e 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
+++ b/drivers/staging/media/atomisp/pci/atomisp_v4l2.c
@@ -711,15 +711,15 @@  static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 
 	dev_dbg(isp->dev, "IUNIT power-%s.\n", enable ? "on" : "off");
 
-	/*WA:Enable DVFS*/
+	/* WA for PUNIT, if DVFS enabled, ISP timeout observed */
 	if (IS_CHT && enable)
-		punit_ddr_dvfs_enable(true);
+		punit_ddr_dvfs_enable(false);
 
 	/*
 	 * FIXME:WA for ECS28A, with this sleep, CTS
 	 * android.hardware.camera2.cts.CameraDeviceTest#testCameraDeviceAbort
 	 * PASS, no impact on other platforms
-	*/
+	 */
 	if (IS_BYT && enable)
 		msleep(10);
 
@@ -727,7 +727,7 @@  static int atomisp_mrfld_power(struct atomisp_device *isp, bool enable)
 	iosf_mbi_modify(BT_MBI_UNIT_PMC, MBI_REG_READ, MRFLD_ISPSSPM0,
 			val, MRFLD_ISPSSPM0_ISPSSC_MASK);
 
-	/*WA:Enable DVFS*/
+	/* WA:Enable DVFS */
 	if (IS_CHT && !enable)
 		punit_ddr_dvfs_enable(true);