linux-kernel - Re: [Freedreno] [PATCH v3 12/13] drm/msm/dsi: Add support for DSC configuration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <Yjr9N4DPdOyi7CO5@matsya>
Date:   Wed, 23 Mar 2022 16:27:59 +0530
From:   Vinod Koul <vkoul@...nel.org>
To:     Marijn Suijten <marijn.suijten@...ainline.org>
Cc:     Jonathan Marek <jonathan@...ek.ca>,
        Jeffrey Hugo <jeffrey.l.hugo@...il.com>,
        David Airlie <airlied@...ux.ie>, linux-arm-msm@...r.kernel.org,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        linux-kernel@...r.kernel.org,
        Abhinav Kumar <abhinavk@...eaurora.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Rob Clark <robdclark@...il.com>,
        dri-devel@...ts.freedesktop.org, Daniel Vetter <daniel@...ll.ch>,
        Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
        freedreno@...ts.freedesktop.org,
        Sumit Semwal <sumit.semwal@...aro.org>
Subject: Re: [Freedreno] [PATCH v3 12/13] drm/msm/dsi: Add support for DSC
 configuration

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