[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALAqxLXxRbYTuP5CaOtzBpX=AeXArP4Td0uZWiVE6=u7kQ9-8g@mail.gmail.com>
Date: Tue, 14 Feb 2017 13:07:21 -0800
From: John Stultz <john.stultz@...aro.org>
To: Daniel Stone <daniel@...ishbar.org>
Cc: lkml <linux-kernel@...r.kernel.org>,
Daniel Vetter <daniel.vetter@...el.com>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Sean Paul <seanpaul@...omium.org>,
David Airlie <airlied@...ux.ie>,
Rob Clark <robdclark@...il.com>,
Xinliang Liu <xinliang.liu@...aro.org>,
Xinliang Liu <z.liuxinliang@...ilicon.com>,
Rongrong Zou <zourongrong@...il.com>,
Xinwei Kong <kong.kongxinwei@...ilicon.com>,
Chen Feng <puck.chen@...ilicon.com>,
Archit Taneja <architt@...eaurora.org>,
dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [RFC][PATCH 1/2] drm/probe-helper: Add mode_valid check to drm_crtc_helper_funcs
On Tue, Feb 14, 2017 at 12:32 PM, Daniel Stone <daniel@...ishbar.org> wrote:
> Hi John,
>
> On 14 February 2017 at 19:25, John Stultz <john.stultz@...aro.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.
thanks
-john
Powered by blists - more mailing lists