linux-kernel - Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper

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]

Message-ID: <CALAqxLV7BAmEswH1FS8w2LLYw_Hfjpa6_RMEOVrK=TfZKnBU3w@mail.gmail.com>
Date:   Mon, 27 Feb 2017 22:03:20 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     lkml <linux-kernel@...r.kernel.org>,
        Chen Feng <puck.chen@...ilicon.com>,
        Xinwei Kong <kong.kongxinwei@...ilicon.com>,
        Xinliang Liu <z.liuxinliang@...ilicon.com>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Rongrong Zou <zourongrong@...il.com>,
        Daniel Vetter <daniel.vetter@...el.com>
Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

On Mon, Feb 20, 2017 at 2:32 PM, Daniel Vetter <daniel@...ll.ch> wrote:
> I thought about this some more, I think what we need to fix this mess
> properly is:
> - mode_valid helper callbacks for crtc, encoder, bridges, with the
> same interface as for connectors.
> - calling all these new mode_valid hooks from the probe helpers, but
> with the restriction that we only reject a mode if all possible
> crtc/encoders combos reject it. We need to filter by
> possible_encoders/crtcs for these checks. Bridges have a fixed routing
> to their encoder, so those are easy.

So... my ignorance here my have complicated things a bit.

Xinliang can correct me here, if I get this wrong. :)

So in the past, when we ran into this issue, we just hacked in a mode
white list into the adv7511 bridge mode_valid check, as it was the
easiest place. But as I understood it, the limitation came from logic
in (what is now an early version of) the kirin ade driver.

However, since kirin made it upstream, the code was refactored and I
believe the specific code was moved to the dsi_calc_phy_rate() which
is part of the dsi encoder, not the crtc code.

So in my limited understanding I initially tried to add the mode_check
logic in the ade crtc driver, but to me (and I could have this wrong
again) it seems it really ought to be in the encoder (where we
probably could do better then using a white list by doing the mode
calculations and seeing if we get a value that matches what was
requested).

I'm not sure if that simplifies your proposal (possibly avoiding the
crtc/encoder combo checks?), or if you think we still need the big
grand solution?

> - add calls to mode_valid in the atomic helpers, right before we call
> mode_fixup or atomic_check in drm_atomic_helper_check_modesets.
> - convert drivers to move code from mode_fixup into mode_valid
> wherever possible, to make sure we can share as much of the check
> logic between probe and atomic comit code.
> - update docs for all the hooks, plus update the overview sections accordingly.
>
> I think this should give us a good approximation of nirvana. For I
> long time I thought we could get away without adding mode_valid
> everywhere, but in the probe paths we really can't fake the
> adjusted_mode (and other atomic state), so adding dedicated hooks
> which are called from both places is probably the only option.

So my mental map of the DRM code is all pretty hazy and limited here,
but the above sounds reasonable. I'll try to trace through and better
understand what specifically you're asking for and get something
started here.

Thanks again for the thoughts and feedback!
-john