On 22-03-22, 19:59, Marijn Suijten wrote: > On 2022-03-22 22:46:50, Vinod Koul wrote: > > On 17-02-22, 16:11, Marijn Suijten wrote: > > > Hi Vinod, > > > > > > Thanks for taking time to go through this review, please find some > > > clarifications below. > > > > > > On 2022-02-17 16:44:04, Vinod Koul wrote: > > > > Hi Marijn, > > > > > > > > On 11-12-21, 01:03, Marijn Suijten wrote: > > > > > > > > > > +static int dsi_dsc_update_pic_dim(struct msm_display_dsc_config *dsc, > > > > > > + int pic_width, int pic_height) > > > > > > > > > > This function - adopted from downstream - does not seem to perform a > > > > > whole lot, especially without the modulo checks against the slice size. > > > > > Perhaps it can be inlined? > > > > > > > > Most of the code here is :) > > > > > > > > This was split from downstream code to check and update dimension. We > > > > can inline this, or should we leave that to compiler. I am not a very > > > > big fan of inlining... > > > > > > It doesn't seem beneficial to code readability to have this function, > > > which is only called just once and also has the same struct members read > > > in a `DBG()` directly, abstracted away to a function. Not really > > > concerned about generated code/performance FWIW. > > > > > > Also note that the caller isn't checking the `-EINVAL` result... > > > > I have made this void inline. > > Perhaps there is a misunderstanding here: with inlining I am referring > to the process of transplanting the _function body_ to the only > call-site, not adding the `inline` keyword nor changing this to `void`. > > The checks that make this function return `-EINVAL` seem valid, so the > caller should check it instead of removing the return? Okay somehow I misunderstood then, let me see how the code looks in this case then -- ~Vinod
