linux-kernel - Re: [PATCH] drm/arcpgu: use .mode_fixup instead of .atomic

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 for Android: free password hash cracker in your pocket

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

Date:   Fri, 3 Mar 2017 19:24:06 +0000
From:   Alexey Brodkin <Alexey.Brodkin@...opsys.com>
To:     Jose Abreu <Jose.Abreu@...opsys.com>
CC:     "dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "daniel@...ll.ch" <daniel@...ll.ch>,
        "daniel.vetter@...ll.ch" <daniel.vetter@...ll.ch>,
        "linux-snps-arc@...ts.infradead.org" 
        <linux-snps-arc@...ts.infradead.org>,
        "airlied@...ux.ie" <airlied@...ux.ie>
Subject: Re: [PATCH] drm/arcpgu: use .mode_fixup instead of .atomic_check

Hi Jose,

On Fri, 2017-03-03 at 18:05 +0000, Jose Abreu wrote:
> Hi Alexey,
> 
> 
> On 03-03-2017 13:27, Alexey Brodkin wrote:
> > 
> > 
> > So if I understood you correct here what I really need is just to get rid of existing check,
> > right? I.e. the following is to be in v2 respin:
> > ------------------------------->8-------------------------------
> > diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > index ad9a95916f1f..86f1555914e8 100644
> > --- a/drivers/gpu/drm/arc/arcpgu_crtc.c
> > +++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
> > @@ -129,20 +129,6 @@ static void arc_pgu_crtc_disable(struct drm_crtc *crtc)
> >                               ~ARCPGU_CTRL_ENABLE_MASK);
> >  }
> >  
> > -static int arc_pgu_crtc_atomic_check(struct drm_crtc *crtc,
> > -                                    struct drm_crtc_state *state)
> > -{
> > -       struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
> > -       struct drm_display_mode *mode = &state->adjusted_mode;
> > -       long rate, clk_rate = mode->clock * 1000;
> > -
> > -       rate = clk_round_rate(arcpgu->clk, clk_rate);
> > -       if (rate != clk_rate)
> > -               return -EINVAL;
> > -
> > -       return 0;
> > -}
> > -
> >  static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
> >                                       struct drm_crtc_state *state)
> >  {
> > @@ -165,7 +151,6 @@ static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
> >         .disable        = arc_pgu_crtc_disable,
> >         .prepare        = arc_pgu_crtc_disable,
> >         .commit         = arc_pgu_crtc_enable,
> > -       .atomic_check   = arc_pgu_crtc_atomic_check,
> >         .atomic_begin   = arc_pgu_crtc_atomic_begin,
> >  };
> > ------------------------------->8-------------------------------
> 
> I don't think you can remove the check entirely as this will make
> any mode be accepted, right?

Correct. Otherwise we'll get some modes and devices that
don't work.

Remember our saga with 74.25 vs 74.40 MHz?

With our PLLs on AXS and HSDK boards we may generate 74.25 MHz clock
which satisfy some monitors especially those who pass correct EDID to the host.
But what if EDID is either corrupted or doesn't exist (that's my case with
some industrial monitor as well as with old DVI monitor)?

In that case Linux kernel attempts to calculate all the values including pixel clock
but then instead of 74.25 we'll get 74.40 and equipment that used to work is no longer useful.

So strictly speaking existing check makes perfect sense. But it reduces
compatibility with not very good monitors.

Probably better solution to the problem is just to throw away [my] faulty HW and
buy equipment that conforms to standards (not really sure if EDID is a hard
requirement for DVI/HDMI displays or this is just an option).

BTW I'm wondering if there're any guidelines on what could be pixel clock
deviation from the requested one?

-Alexey