[PATCH 2/3] staging: media: max96712: enable extra debug register for max96724

Laurentiu Palcu posted 3 patches 1 year ago
There is a newer version of this series
[PATCH 2/3] staging: media: max96712: enable extra debug register for max96724
Posted by Laurentiu Palcu 1 year ago
The current implementation has the register disabled since it is not
documented in the MAX96724 RM. However, in the chip's user's guide [1],
in the Video Pattern Generator section, the register is documented and
it appears to work just fine. Before the change I was experiencing
approx 10fps when streaming, after: 30.3fps.

[1] https://www.analog.com/media/en/technical-documentation/user-guides/max96724fr-user-guide.pdf

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
---
 drivers/staging/media/max96712/max96712.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
index 5228f9ec60859..682ebd20851f7 100644
--- a/drivers/staging/media/max96712/max96712.c
+++ b/drivers/staging/media/max96712/max96712.c
@@ -462,6 +462,7 @@ static const struct max96712_info max96712_info_max96712 = {
 
 static const struct max96712_info max96712_info_max96724 = {
 	.dpllfreq = 1200,
+	.have_debug_extra = true,
 };
 
 static const struct of_device_id max96712_of_table[] = {
-- 
2.44.1
Re: [PATCH 2/3] staging: media: max96712: enable extra debug register for max96724
Posted by Niklas Söderlund 12 months ago
Hello Laurentiu,

Thanks for your work.

On 2024-12-19 17:06:42 +0200, Laurentiu Palcu wrote:
> The current implementation has the register disabled since it is not
> documented in the MAX96724 RM. However, in the chip's user's guide [1],
> in the Video Pattern Generator section, the register is documented and
> it appears to work just fine. Before the change I was experiencing
> approx 10fps when streaming, after: 30.3fps.
> 
> [1] https://www.analog.com/media/en/technical-documentation/user-guides/max96724fr-user-guide.pdf
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> ---
>  drivers/staging/media/max96712/max96712.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> index 5228f9ec60859..682ebd20851f7 100644
> --- a/drivers/staging/media/max96712/max96712.c
> +++ b/drivers/staging/media/max96712/max96712.c
> @@ -462,6 +462,7 @@ static const struct max96712_info max96712_info_max96712 = {
>  
>  static const struct max96712_info max96712_info_max96724 = {
>  	.dpllfreq = 1200,
> +	.have_debug_extra = true,

This flag was added as I could not find the register in the MAX96724 
datasheet I had access to at the time. As you now prove the register 
exists in the user's guide and that it works I think a better solution 
is to remove the 'have_debug_extra' flag all together then set it for 
both supported devices.

>  };
>  
>  static const struct of_device_id max96712_of_table[] = {
> -- 
> 2.44.1
> 

-- 
Kind Regards,
Niklas Söderlund
Re: [PATCH 2/3] staging: media: max96712: enable extra debug register for max96724
Posted by Laurentiu Palcu 12 months ago
Hi Niklas,

On Thu, Dec 19, 2024 at 06:09:23PM +0100, Niklas Söderlund wrote:
> Hello Laurentiu,
> 
> Thanks for your work.
> 
> On 2024-12-19 17:06:42 +0200, Laurentiu Palcu wrote:
> > The current implementation has the register disabled since it is not
> > documented in the MAX96724 RM. However, in the chip's user's guide [1],
> > in the Video Pattern Generator section, the register is documented and
> > it appears to work just fine. Before the change I was experiencing
> > approx 10fps when streaming, after: 30.3fps.
> > 
> > [1] https://www.analog.com/media/en/technical-documentation/user-guides/max96724fr-user-guide.pdf
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
> > ---
> >  drivers/staging/media/max96712/max96712.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/staging/media/max96712/max96712.c b/drivers/staging/media/max96712/max96712.c
> > index 5228f9ec60859..682ebd20851f7 100644
> > --- a/drivers/staging/media/max96712/max96712.c
> > +++ b/drivers/staging/media/max96712/max96712.c
> > @@ -462,6 +462,7 @@ static const struct max96712_info max96712_info_max96712 = {
> >  
> >  static const struct max96712_info max96712_info_max96724 = {
> >  	.dpllfreq = 1200,
> > +	.have_debug_extra = true,
> 
> This flag was added as I could not find the register in the MAX96724 
> datasheet I had access to at the time. As you now prove the register 
> exists in the user's guide and that it works I think a better solution 
> is to remove the 'have_debug_extra' flag all together then set it for 
> both supported devices.

Ok, makes sense. I guess I was thinking of having this driver support
other chips as well in the future and that's why I left it there, for
chip variants that don't actually have it. But, on the other hand, it
could always be added later if needed.

I'll send a v2 with the flag removed.

Thanks,
Laurentiu

> 
> >  };
> >  
> >  static const struct of_device_id max96712_of_table[] = {
> > -- 
> > 2.44.1
> > 
> 
> -- 
> Kind Regards,
> Niklas Söderlund