On Tue, Dec 17, 2019 at 11:27 AM Stephan Gerhold <stephan@xxxxxxxxxxx> wrote: > I feel kind of bad to keep requesting changes for this patch, Don't feel like that. It is complex hardware and complex code, so it leads to complex development. Also I am making way too many stupid mistakes :/ > > + val = readl(d->regs + DSI_VID_PCK_TIME); > > + val &= ~DSI_VID_PCK_TIME_BLKEOL_DURATION_MASK; > > + val |= blkeol_duration << > > + DSI_VID_PCK_TIME_BLKEOL_DURATION_SHIFT; > > + writel(val, d->regs + DSI_VID_PCK_TIME); > > + > > + /* Max burst limit, this is given in bytes */ > > + val = readl(d->regs + DSI_VID_VCA_SETTING1); > > + val &= ~DSI_VID_VCA_SETTING1_MAX_BURST_LIMIT_MASK; > > + val |= blkeol_duration - 6; > > The vendor kernel writes blkeol_pck - 6 (instead of blkeol_duration) here: > > dsi_wfld(io, DSI_VID_VCA_SETTING1, MAX_BURST_LIMIT, vid_regs->blkeol_pck - 6); You're right, and still I read the code over and over... It's good we have 2 pairs of eyes. > Also: It does not make a functional difference here but for clarity we > should shift the value by DSI_VID_VCA_SETTING1_MAX_BURST_LIMIT_SHIFT (= 0), > i.e. > > val |= blkeol_pck - 6 << DSI_VID_VCA_SETTING1_MAX_BURST_LIMIT_SHIFT; OK I fix! Yours, Linus Walleij _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel