linux-kernel - Re: [PATCH v2 2/8] drm: Add drm_crtc_mode

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:   Wed, 10 May 2017 17:12:58 +0200
From:   Daniel Vetter <daniel@...ll.ch>
To:     Jose Abreu <Jose.Abreu@...opsys.com>
Cc:     Daniel Vetter <daniel.vetter@...ll.ch>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Carlos Palminha <CARLOS.PALMINHA@...opsys.com>,
        Alexey Brodkin <Alexey.Brodkin@...opsys.com>,
        Ville Syrjälä 
        <ville.syrjala@...ux.intel.com>, Dave Airlie <airlied@...ux.ie>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Archit Taneja <architt@...eaurora.org>
Subject: Re: [PATCH v2 2/8] drm: Add drm_crtc_mode_valid()

On Wed, May 10, 2017 at 09:57:30AM +0100, Jose Abreu wrote:
> Hi Daniel,
> 
> 
> On 10-05-2017 08:59, Daniel Vetter wrote:
> > On Tue, May 09, 2017 at 06:00:09PM +0100, Jose Abreu wrote:
> >> Add a new helper to call crtc->mode_valid callback.
> >>
> >> Suggested-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> >> Signed-off-by: Jose Abreu <joabreu@...opsys.com>
> >> Cc: Carlos Palminha <palminha@...opsys.com>
> >> Cc: Alexey Brodkin <abrodkin@...opsys.com>
> >> Cc: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> >> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> >> Cc: Dave Airlie <airlied@...ux.ie>
> >> Cc: Andrzej Hajda <a.hajda@...sung.com>
> >> Cc: Archit Taneja <architt@...eaurora.org>
> >> ---
> >>  drivers/gpu/drm/drm_crtc.c          | 22 ++++++++++++++++++++++
> >>  drivers/gpu/drm/drm_crtc_internal.h |  3 +++
> >>  2 files changed, 25 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> >> index 5af25ce..07ae705 100644
> >> --- a/drivers/gpu/drm/drm_crtc.c
> >> +++ b/drivers/gpu/drm/drm_crtc.c
> >> @@ -38,6 +38,7 @@
> >>  #include <drm/drm_crtc.h>
> >>  #include <drm/drm_edid.h>
> >>  #include <drm/drm_fourcc.h>
> >> +#include <drm/drm_modeset_helper_vtables.h>
> >>  #include <drm/drm_modeset_lock.h>
> >>  #include <drm/drm_atomic.h>
> >>  #include <drm/drm_auth.h>
> >> @@ -741,3 +742,24 @@ int drm_mode_crtc_set_obj_prop(struct drm_mode_object *obj,
> >>  
> >>  	return ret;
> >>  }
> >> +
> >> +/**
> >> + * drm_crtc_mode_valid - call crtc->mode_valid callback, if any.
> >> + * @crtc: crtc
> >> + * @mode: mode to be validated
> >> + *
> >> + * If no mode_valid callback is available this will return MODE_OK.
> >> + *
> >> + * Returns: drm_mode_status Enum
> >> + */
> >> +enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc,
> >> +					 const struct drm_display_mode *mode)
> >> +{
> >> +	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> > This is clearly a helper func, but you place it into the core and
> > EXPORT_SYMBOL it. Imo this should be entirely internal to the helpers,
> > perhaps just stuff them all into drm_probe_helpers.c? Header file would be
> > drm_crtc_helper_internal.h.
> 
> Yeah, at first I was not planning to export it but then I saw
> that drm_bridge_mode_fixup() is exported (and is in drm_bridge.c)
> so it kind of felt right to place this in drm_crtc.c. Anyway, I
> will move them to drm_probe_helpers.c, indeed there is no point
> in exporting this.

Bridge is a bit special, since there's the bridge integration through
atomic/legacy helpers, but drivers could also wire up a bridge on their
own (i.e. without using the drm_encoder->bridge pointer). Not sure anyone
is doing that right now, but that was the idea behind having the helpers
all exported.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch