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

[Laurent Pinchart]

On Wed, Nov 06, 2024 at 10:20:43AM +0000, Biju Das wrote:
> 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.

Is that an issue specific to that board, or to the chip in general ? If
it's specific to the board, disabling 1 lane usage for everybody in the
driver isn't the right option.

> > 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??
> 
> > > 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