Re: [PATCH 01/76] drm/fb-helper: don't clobber output routing in setup_crtcs

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Thu, 26 Jul 2012 20:48:26 +0200
Daniel Vetter <daniel.vetter@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@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@virtuousgeek.org> fwiw.