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

Groups > linux.kernel > #1580891

Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs

Show all headers | View raw

On Tue, Feb 14, 2017 at 01:07:21PM -0800, John Stultz wrote:
> On Tue, Feb 14, 2017 at 12:32 PM, Daniel Stone <daniel@fooishbar.org> wrote:
> > Hi John,
> >
> > On 14 February 2017 at 19:25, John Stultz <john.stultz@linaro.org> wrote:
> >> +static enum drm_mode_status
> >> +drm_connector_check_crtc_modes(struct drm_connector *connector,
> >> +                              struct drm_display_mode *mode)
> >> +{
> >> +       struct drm_device *dev = connector->dev;
> >> +       const struct drm_crtc_helper_funcs *crtc_funcs;
> >> +       struct drm_crtc *c;
> >> +
> >> +       if (mode->status != MODE_OK)
> >> +               return mode->status;
> >> +
> >> +       /* Check all the crtcs on a connector to make sure the mode is valid */
> >> +       drm_for_each_crtc(c, dev) {
> >> +               crtc_funcs = c->helper_private;
> >> +               if (crtc_funcs && crtc_funcs->mode_valid)
> >> +                       mode->status = crtc_funcs->mode_valid(c, mode);
> >> +               if (mode->status != MODE_OK)
> >> +                       break;
> >> +       }
> >> +       return mode->status;
> >> +}
> >
> > Hm, that's unfortunate: it limits the mode list for every connector,
> > to those which are supported by every single CRTC. So if you have one
> > CRTC serving low-res LVDS, and another serving higher-res HDMI,
> > suddenly you can't get bigger modes on HDMI. The idea seems sound
> > enough, but a little more nuance might be good ...
> 
> Yea. That is not my intent at all I'm just trying to get the drm_crtc
> attached to the connector that we're getting the EDID mode lines from.
> I had tried going connector->encoder->crtc, but at the time this is
> called, the encoder is null. So Rob suggested the for_each_crtc(), and
> I guess I mistook that for being each crtc on the connector.
> 
> Thanks for pointing out this issue. From Daniel's feedback it looks
> like I need to start over from scratch though, so little worry this
> implementation will go much further.

Well your idea was somewhat right, but logic inverted. In ->mode_valid we
need to check whether any encoder/crtc combo could support the mode. Which
means you need to reject it only when there's no encoder/crtc combo that
could support the mode (you reject it if there's only one crtc which can't
handle it).

On the ->mode_fixup/atomic_check callbacks otoh we need to reject the mode
when it's not suitable for the current chain (as described in the atomic
states). That little difference is why this is not an entirely trivial
problem, and yes there's lots of hw out there where this matters.
-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

[RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs John Stultz <john.stultz@linaro.org> - 2017-02-14 20:30 +0100
  Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Daniel Vetter <daniel@ffwll.ch> - 2017-02-14 20:40 +0100
    Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs John Stultz <john.stultz@linaro.org> - 2017-02-14 20:50 +0100
      Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to  drm_crtc_helper_funcs Daniel Vetter <daniel@ffwll.ch> - 2017-02-14 21:30 +0100
        Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs John Stultz <john.stultz@linaro.org> - 2017-02-14 22:10 +0100
          Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to  drm_crtc_helper_funcs Daniel Vetter <daniel@ffwll.ch> - 2017-02-14 22:50 +0100
            Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to  drm_crtc_helper_funcs Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-02-15 18:00 +0100
    Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to  drm_crtc_helper_funcs Ville Syrjälä <ville.syrjala@linux.intel.com> - 2017-02-14 21:00 +0100
  Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Daniel Stone <daniel@fooishbar.org> - 2017-02-14 21:40 +0100
    Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs John Stultz <john.stultz@linaro.org> - 2017-02-14 22:10 +0100
      Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to  drm_crtc_helper_funcs Daniel Vetter <daniel@ffwll.ch> - 2017-02-14 22:50 +0100
        Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Rob Clark <robdclark@gmail.com> - 2017-02-15 03:30 +0100
  Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs Daniel Vetter <daniel@ffwll.ch> - 2017-02-20 23:40 +0100
    Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs John Stultz <john.stultz@linaro.org> - 2017-02-28 07:30 +0100
      Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to  drm_crtc_helper_funcs Daniel Vetter <daniel@ffwll.ch> - 2017-02-28 10:40 +0100

csiph-web