On Thu, 26 Jul 2012 20:48:26 +0200
Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Yet again the too close relationship between the fb helper and the
> crtc helper code strikes. This time around the fb helper resets all
> encoder->crtc pointers to NULL before starting to set up it's own
> mode. Which is total bullocks, because this will clobber the existing
> output routing, which the new drm/i915 code depends upon to be
> absolutely correct.
>
> The crtc helper itself doesn't really care about that, since it
> disables unused encoders in a rather roundabout way.
>
> Two places call drm_setup_crts:
>
> - For the initial fb config. I've auditted all current drivers, and
> they all allocate their encoders with kzalloc. So there's no need to
> clear encoder->crtc once more.
>
> - When processing hotplug events while showing the fb console. This
> one is a bit more tricky, but both the crtc helper code and the new
> drm/i915 modeset code disable encoders if their crtc is changed and
> that encoder isn't part of the new config. Also, both disable any
> disconnected outputs, too.
>
> Which only leaves encoders that are on, connected, but for some odd
> reason the fb helper code doesn't want to use. That would be a bug
> in the fb configuration selector, since it tries to light up as many
> outputs as possible.
>
> v2: Kill the now unused encoders variable.
>
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/drm_fb_helper.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index f546d1e..4ecc869 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1230,7 +1230,6 @@ static void drm_setup_crtcs(struct
> drm_fb_helper *fb_helper) struct drm_device *dev = fb_helper->dev;
> struct drm_fb_helper_crtc **crtcs;
> struct drm_display_mode **modes;
> - struct drm_encoder *encoder;
> struct drm_mode_set *modeset;
> bool *enabled;
> int width, height;
> @@ -1241,11 +1240,6 @@ static void drm_setup_crtcs(struct
> drm_fb_helper *fb_helper) width = dev->mode_config.max_width;
> height = dev->mode_config.max_height;
>
> - /* clean out all the encoder/crtc combos */
> - list_for_each_entry(encoder, &dev->mode_config.encoder_list,
> head) {
> - encoder->crtc = NULL;
> - }
> -
> crtcs = kcalloc(dev->mode_config.num_connector,
> sizeof(struct drm_fb_helper_crtc *),
> GFP_KERNEL); modes = kcalloc(dev->mode_config.num_connector,
I remember running into this when doing the fastboot stuff, but I
thought it had other side effects too (maybe hot plug). But that was
probably some other bug I worked through in while testing, since in
theory the above should be ok.
All of this stuff will need lots of tested-bys, but this one has my
Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org> fwiw.
