Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1637282

Re: [PATCH 4/5] drm: Use mode_valid() in atomic modeset

Show all headers | View raw

On Thu, May 04, 2017 at 04:13:51PM +0100, Jose Abreu wrote:
> On 04-05-2017 15:40, Ville Syrjälä wrote:
> > On Thu, May 04, 2017 at 03:11:41PM +0100, Jose Abreu wrote:
> >> +					    struct drm_encoder *encoder,
> >> +					    struct drm_crtc *crtc,
> >> +					    struct drm_display_mode *mode)
> >> +{
> >> +	const struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
> >> +	const struct drm_connector_helper_funcs *connector_funcs =
> >> +		connector->helper_private;
> >> +	const struct drm_encoder_helper_funcs *encoder_funcs =
> >> +		encoder->helper_private;
> >> +	enum drm_mode_status ret = MODE_OK;
> >> +
> >> +	if (connector_funcs && connector_funcs->mode_valid)
> >> +		ret = connector_funcs->mode_valid(connector, mode);
> > As I mentioned earlier, this would break i915. We either can't call the 
> > connector hook at all here, or we have to make it somehow opt-in if some
> > drivers really want to do it.
> 
> Ok. You said you let users exceed the limits, but why doesn't it
> fail in atomic_check and will fail in mode_valid? I guess I can
> remove it, but this can lead to lots of confusion, i.e. with this
> series the mode_valid callbacks for all components are called, if
> I remove just one call the whole point will fall apart.
> 
> Can we think about something else ? I think making it opt-in is
> not ideal, if the helper is there then it should be used, but if
> thats the best solution then shall we add a flag which will call
> or not *all* the mode_valid callbacks in this stage
> (drm_device.allow_modevalid_call, ...) ?

This is a general issue, not a driver opt-in. With your new callbacks
ideally we'll have:
- all the source checks (clock limits, size limits) are in the
  crtc/encoder/bridge callbacks
- only the sink limit checks (derived from edid or DP aux) are in the
  connector callback

Letting userspace overwrite these seems like a reasonable idea that we
should support in general I think. See also my comment on patch 1 for the
connector's mode_valid kerneldoc.

For arcpgu that might mean that you need to move part of the connector's
mode_valid checks into the encoder, but since all the encoder modeset is
in there already, that seems like a good cleanup of the drm modeset
framework.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 0/5] Introduce new mode validation callbacks Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 16:20 +0200
  [PATCH 4/5] drm: Use mode_valid() in atomic modeset Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 16:20 +0200
    Re: [PATCH 4/5] drm: Use mode_valid() in atomic modeset Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-05-04 16:50 +0200
      Re: [PATCH 4/5] drm: Use mode_valid() in atomic modeset Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 17:20 +0200
        Re: [PATCH 4/5] drm: Use mode_valid() in atomic modeset Daniel Vetter <daniel@ffwll.ch> - 2017-05-08 10:00 +0200
  [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 16:20 +0200
    Re: [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe  helper Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-05-04 16:40 +0200
      Re: [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe  helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 17:00 +0200
    Re: [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe  helper Daniel Vetter <daniel@ffwll.ch> - 2017-05-08 10:00 +0200
      Re: [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe  helper Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-08 12:20 +0200
        Re: [PATCH 2/5] drm: Use new mode_valid() helpers in connector probe  helper Daniel Vetter <daniel@ffwll.ch> - 2017-05-08 20:30 +0200
  [PATCH 3/5] drm: Introduce drm_bridge_mode_valid() Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 16:20 +0200
  [PATCH 5/5] drm: arc: Use crtc->mode_valid() callback Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 16:20 +0200
  [PATCH 1/5] drm: Add crtc/encoder/bridge->mode_valid() callbacks Jose Abreu <Jose.Abreu@synopsys.com> - 2017-05-04 16:20 +0200
    Re: [PATCH 1/5] drm: Add crtc/encoder/bridge->mode_valid() callbacks Daniel Vetter <daniel@ffwll.ch> - 2017-05-08 10:00 +0200
  Re: [PATCH 0/5] Introduce new mode validation callbacks Daniel Vetter <daniel@ffwll.ch> - 2017-05-08 10:00 +0200

csiph-web