[PATCH v2 2/2] drm: adv7511: Fix out-of-bounds array in clock_div_by_lanes

[Biju Das]

Hi Laurent Pinchart,

Thanks for the feedback.

> -----Original Message-----
> From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Sent: 05 November 2024 16:06
> Subject: Re: [PATCH v2 2/2] drm: adv7511: Fix out-of-bounds array in clock_div_by_lanes
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Tue, Nov 05, 2024 at 11:12:19AM +0000, Biju Das wrote:
> > Fix out-of-bounds array in adv7511_dsi_config_timing_gen(), when dsi
> > lanes = 1.
> 
> Does the hardware support using the internal timing generator with a single lane ? If so

As per the binding documentation [1], ADV7535 supports single lane.
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/Documentation/devicetree/bindings/display/bridge/adi,adv7533.yaml?h=next-20241106



> adv7511_dsi_config_timing_gen() should be fixed, otherwise that should be explained in the commit

On RZ/G2L SMARC EVK platform, lanes=2,3,4 works ok, But with setting lanes=1, video is unstable
by trying with clock_divider as 6,7 and 8 by updating the array and also disabling internal timing generator.


> message, and mentioned with a comment in adv7533_parse_dt(). I would also print an error message in
> that case.

OK, this can be done.



> 
> If the internal timing generator can't be used with a single lane, the DT bindings should also be
> updated to document that.

Even single lane with or without internal timing generator does not work on RZ/G2L.

So, any users of ADV 7535 tested single lane??

Cheers,
Biju

> 
> > Fixes: 78fa479d703c ("drm/bridge: adv7533: Use internal timing
> > generator")
> > Reported-by: Hien Huynh <hien.huynh.px at renesas.com>
> > Signed-off-by: Biju Das <biju.das.jz at bp.renesas.com>
> > ---
> > Changes in v2:
> >  - Added the tag "Cc: stable at vger.kernel.org" in the sign-off area.
> >  - Dropped Archit Taneja invalid Mail address
> > ---
> >  drivers/gpu/drm/bridge/adv7511/adv7533.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > index 3e57ba838e5e..0c2236e53af5 100644
> > --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
> > @@ -185,6 +185,9 @@ int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv)
> >  	adv->use_timing_gen = !of_property_read_bool(np,
> >  						"adi,disable-timing-generator");
> >
> > +	if (adv->use_timing_gen && num_lanes == 1)
> > +		return -EINVAL;
> > +
> >  	/* TODO: Check if these need to be parsed by DT or not */
> >  	adv->rgb = true;
> >  	adv->embedded_sync = false;
> 
> --
> Regards,
> 
> Laurent Pinchart