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]

Date:   Tue, 14 Feb 2017 21:51:29 +0200
From:   Ville Syrjälä <ville.syrjala@...ux.intel.com>
To:     Daniel Vetter <daniel@...ll.ch>
Cc:     John Stultz <john.stultz@...aro.org>,
        Chen Feng <puck.chen@...ilicon.com>,
        lkml <linux-kernel@...r.kernel.org>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        Xinliang Liu <z.liuxinliang@...ilicon.com>,
        Xinwei Kong <kong.kongxinwei@...ilicon.com>,
        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 Tue, Feb 14, 2017 at 08:38:40PM +0100, Daniel Vetter wrote:
> On Tue, Feb 14, 2017 at 8:25 PM, John Stultz <john.stultz@...aro.org> wrote:
> > Currently, on the hikey board, we have the adv7511 bridge wired
> > up to the kirin ade drm driver. Unfortunately, the kirin ade
> > core cannot generate accurate byteclocks for all pixel clock
> > values.
> >
> > Thus if a mode clock is selected that we cannot calculate a
> > matching byteclock, the device will boot with a blank screen.
> >
> > Unfortunately, currently the only place we can properly check
> > potential modes for this issue in the connector mode_valid
> > helper. Again, hikey uses the adv7511 bridge, which is shared
> > between a number of different devices, so its improper to put
> > restrictions caused by the kirin drm driver in the adv7511
> > logic.
> >
> > So this patch tries to correct for that, by adding some
> > infrastructure so that the drm_crtc_helper_funcs can optionally
> > implement a mode_valid check, so that the probe helpers can
> > check to make sure there are not any restrictions at the crtc
> > level as well.
> >
> > Cc: Daniel Vetter <daniel.vetter@...el.com>
> > Cc: Jani Nikula <jani.nikula@...ux.intel.com>
> > Cc: Sean Paul <seanpaul@...omium.org>
> > Cc: David Airlie <airlied@...ux.ie>
> > Cc: Rob Clark <robdclark@...il.com>
> > Cc: Xinliang Liu <xinliang.liu@...aro.org>
> > Cc: Xinliang Liu <z.liuxinliang@...ilicon.com>
> > Cc: Rongrong Zou <zourongrong@...il.com>
> > Cc: Xinwei Kong <kong.kongxinwei@...ilicon.com>
> > Cc: Chen Feng <puck.chen@...ilicon.com>
> > Cc: Archit Taneja <architt@...eaurora.org>
> > Cc: dri-devel@...ts.freedesktop.org
> > Signed-off-by: John Stultz <john.stultz@...aro.org>
> 
> So I'm going to be super-annoying here and ask for a complete
> solution. This here is defacto what ever driver already does (or has
> too), but it doesn't really solve the overall issue of having entirely
> separate validation paths for probe and atomic_check paths. I think if
> we wan to solve this, we need to solve this properly, with a generic
> solution. That would mean:
> - still in helpers, to make it all opt-int
> - covers crtc and encoders and bridges
> - allows you to implement the current mode_valid in terms of the new
> stuff (maybe as a default hook)
> - allows you to implement the current assortment of mode_fixup and/or
> atomic_check in terms of the new stuff, or at least to not have to
> duplicate logic in there

Long ago I quickly looked at doing this for i915. IIRC the main
complication was the normal vs. the crtc_ timings stored in the
mode. I suppose one option would be to populate the crtc_ timings
in .mode_valid() as well and otherwise just ignore the normal timings.

-- 
Ville Syrjälä
Intel OTC