[PATCH v2 2/8] drm: Add drm_crtc_mode_valid()

[Daniel Vetter]

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 at linux.intel.com>
> >> Signed-off-by: Jose Abreu <joabreu at synopsys.com>
> >> Cc: Carlos Palminha <palminha at synopsys.com>
> >> Cc: Alexey Brodkin <abrodkin at synopsys.com>
> >> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
> >> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >> Cc: Dave Airlie <airlied at linux.ie>
> >> Cc: Andrzej Hajda <a.hajda at samsung.com>
> >> Cc: Archit Taneja <architt at codeaurora.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