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: <20170214214253.GE11138@dvetter-linux.ger.corp.intel.com>
Date:   Tue, 14 Feb 2017 22:42:53 +0100
From:   Daniel Vetter <daniel@...ll.ch>
To:     John Stultz <john.stultz@...aro.org>
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>,
        Daniel Vetter <daniel@...ll.ch>
Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to
 drm_crtc_helper_funcs

On Tue, Feb 14, 2017 at 01:03:27PM -0800, John Stultz wrote:
> On Tue, Feb 14, 2017 at 12:22 PM, Daniel Vetter <daniel@...ll.ch> wrote:
> > On Tue, Feb 14, 2017 at 11:45:54AM -0800, John Stultz wrote:
> >>
> >> Not following here. What do you mean by "put it into drivers"?  Where?
> >
> > In your driver callback for ->mode_valid, call into a shared function to
> > validate CRTC limits. Which you also call from the crtc's ->mode_fixup
> > function.
> 
> So bascially have the adv7511 driver's mode_valid() have a special
> callback to the kirin (and freedreno, and whatever other) drm driver
> to check the modes? Or I guess the drm driver that uses that bridge
> should register something w/ the adv7511 code?
> 
> Part of my confusion here is that the main issue is that its not just
> one driver I'm dealing with, and they're all really just tied together
> via device tree, so I'm not sure how to special case it in just one of
> the drivers.

This sounds you want to fix this for bridges, but your patch only adds a
crtc callback?

> > In short my complain here is that this is only a partial solution of the
> > larger problem, specific for your driver. That kind of code is better put
> > into drivers, until we have a clear understanding to type up something
> > complete in the helpers. Shared code is imo overrated :-)
> 
> Yea, apologies for my not seeing the larger problem at first (its
> still a bit hazy, really), part of this submission is just trying to
> get something to throw darts at and figure out the right thing.
> 
> But I'll try to figure out another approach here.

Latest kerneldoc in drm-tip should explain this, not sure you didn't spot
it or looked at an old kernel version. For drm docs, _always_ look at
drm-tip, they're moving real fast :-)

https://dri.freedesktop.org/docs/drm/gpu/drm-kms-helpers.html#modeset-helper-reference-for-common-vtables

See e.g. drm_crtc_helpers_funcs->mode_fixup docs, it's there. There's
explanations sprinkled at various places (mode_valid, plus the different
helper funcs), probably good to first hunt these all down. Then read a
bunch of driver callbacks so you have a better idea of the patterns of
checks, otoh _lots_ of kms drivers get this wrong :(

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch