Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/imx: dc-ed: invalid link IDs accepted in dc_ed_pec_src_sel_get() --
commit e94375137aef3b831a8ffe20e951e2c25abaed0f Author: Liu Ying <[email protected]> drm/imx: dc-ed: Support getting source selection This commit adds dc_ed_pec_src_sel_get() to read and validate the ExtDst source selection from PIXENGCFG_DYNAMIC. > diff --git a/drivers/gpu/drm/imx/dc/dc-ed.c b/drivers/gpu/drm/imx/dc/dc-ed.c > index de1b71315eabd..67163dca1363c 100644 > --- a/drivers/gpu/drm/imx/dc/dc-ed.c > +++ b/drivers/gpu/drm/imx/dc/dc-ed.c > @@ -149,6 +149,35 @@ void dc_ed_pec_src_sel(struct dc_ed *ed, enum dc_link_id > src) > } > } > > +int dc_ed_pec_src_sel_get(struct dc_ed *ed, enum dc_link_id *src) > +{ > + u32 val; > + int ret; > + > + ret = regmap_read(ed->reg_pec, PIXENGCFG_DYNAMIC, &val); > + if (ret) > + return -EINVAL; > + > + switch (val) { > + case LINK_ID_NONE: > + case LINK_ID_CONSTFRAME0: > + case LINK_ID_CONSTFRAME4: > + case LINK_ID_CONSTFRAME1: > + case LINK_ID_CONSTFRAME5: > + case LINK_ID_FETCHWARP2: > + case LINK_ID_FETCHLAYER0: > + case LINK_ID_LAYERBLEND0: > + case LINK_ID_LAYERBLEND1: > + case LINK_ID_LAYERBLEND2: > + case LINK_ID_LAYERBLEND3: > + *src = val; > + dev_dbg(ed->dev, "get source selection: 0x%02x\n", val); > + return 0; > + default: > + return -EINVAL; > + } > +} [Severity: Medium] Should dc_ed_pec_src_sel_get() validate the value read from PIXENGCFG_DYNAMIC against src_sels[] rather than accepting LINK_ID_FETCHWARP2 and LINK_ID_FETCHLAYER0? In drivers/gpu/drm/imx/dc/dc-ed.c, src_sels[] defines the valid source link IDs for the ExtDst unit and is used by dc_ed_pec_src_sel(): static const enum dc_link_id src_sels[] = { LINK_ID_NONE, LINK_ID_CONSTFRAME0, LINK_ID_CONSTFRAME1, LINK_ID_CONSTFRAME4, LINK_ID_CONSTFRAME5, LINK_ID_LAYERBLEND3, LINK_ID_LAYERBLEND2, LINK_ID_LAYERBLEND1, LINK_ID_LAYERBLEND0, }; void dc_ed_pec_src_sel(struct dc_ed *ed, enum dc_link_id src) { int i; for (i = 0; i < ARRAY_SIZE(src_sels); i++) { if (src_sels[i] == src) { regmap_write(ed->reg_pec, PIXENGCFG_DYNAMIC, src); return; } } } Because fetch units only connect to LayerBlend secondary inputs (sec_sels[] in dc-lb.c), LINK_ID_FETCHWARP2 and LINK_ID_FETCHLAYER0 are rejected by dc_ed_pec_src_sel(). However, dc_ed_pec_src_sel_get() accepts them and returns 0 instead of -EINVAL, which could cause callers such as dc_crtc_disable_at_boot() to treat an invalid ExtDst source as an active pipeline. Would it be better to iterate over src_sels[] in dc_ed_pec_src_sel_get() just like dc_ed_pec_src_sel() does? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=10
