linux-kernel - Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to DP bridge driver

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] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 24 Apr 2020 20:12:04 +0800
From:   Nicolas Boichat <drinkcat@...omium.org>
To:     Xin Ji <xji@...logixsemi.com>
Cc:     devel@...verdev.osuosl.org,
        Laurent Pinchart <laurent.pinchart@...asonboard.com>,
        Dan Carpenter <dan.carpenter@...cle.com>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Neil Armstrong <narmstrong@...libre.com>,
        Jonas Karlman <jonas@...boo.se>,
        Jernej Skrabec <jernej.skrabec@...l.net>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        lkml <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Pi-Hsun Shih <pihsun@...omium.org>,
        Sheng Pan <span@...logixsemi.com>
Subject: Re: [PATCH v7 2/2] drm/bridge: anx7625: Add anx7625 MIPI DSI/DPI to
 DP bridge driver

On Fri, Apr 24, 2020 at 2:51 PM Xin Ji <xji@...logixsemi.com> wrote:
>
> On Thu, Apr 23, 2020 at 07:55:15PM +0800, Nicolas Boichat wrote:
> > Hi,
> >
> > Just commenting on the mode_fixup function that was added in v7.
> >
> [snip]
> > > +       /*
> > > +        * once illegal timing detected, use default HFP, HSYNC, HBP
> > > +        */
> > > +       if (hblanking < HBLANKING_MIN || (hfp < HP_MIN && hbp < HP_MIN)) {
> >
> > should this be adj_hblanking/adj_hfp/adj_hbp?
> NO, need check original HFP and HBP, if they are not legal, driver need
> set default value to adj_hsync, adj_hfp, adj_hbp.
> >
> > > +               adj_hsync = SYNC_LEN_DEF;
> > > +               adj_hfp = HFP_HBP_DEF;
> > > +               adj_hbp = HFP_HBP_DEF;
> > > +               vref = adj->clock * 1000 / (adj->htotal * adj->vtotal);
> > > +               if (hblanking < HBLANKING_MIN) {
> > > +                       delta_adj = HBLANKING_MIN - hblanking;
> > > +                       adj_clock = vref * delta_adj * adj->vtotal;
> > > +                       adj->clock += DIV_ROUND_UP(adj_clock, 1000);
> > > +               } else {
> > > +                       delta_adj = hblanking - HBLANKING_MIN;
> > > +                       adj_clock = vref * delta_adj * adj->vtotal;
> > > +                       adj->clock -= DIV_ROUND_UP(adj_clock, 1000);
> > > +               }
> > > +
> > > +               DRM_WARN("illegal hblanking timing, use default.\n");
> > > +               DRM_WARN("hfp(%d),hbp(%d),hsync(%d).\n", hfp, hbp, hsync);
> >
> > How likely is it that this mode is going to work? Can you just return
> > false here to reject the mode?
> We want to set the default minimal Hblancking value, then it may display,
> otherwise. If we just return false, there is no display for sure.

Right, understand your argument. I'm pondering if it's not just better
to reject the mode rather than trying a timing that is definitely
quite different from what the monitor was asking for. No super strong
opinion, I'll let other people on the list weigh in.