Am 07.02.20 um 14:59 schrieb Ville Syrjala: > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Many drivers are populating encoder->possible_clones wrong. Let's > persuade them to get it right by adding some loud WARNs. > > We'll cross check the bits between any two encoders. So either > both encoders can clone with the other, or neither can. > > We'll also complain about effectively empty possible_clones, and > possible_clones containing bits for encoders that don't exist. > > TODO: Or should we just silently filter out any bits for non-existing > encoders? Please be noisy. Setting these masks incorrectly could be a bug. > > v2: encoder->possible_clones now includes the encoder itelf > > Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> > Cc: Daniel Vetter <daniel@xxxxxxxx> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Acked-by: Thomas Zimmermann <tzimmermann@xxxxxxx> > --- > drivers/gpu/drm/drm_encoder.c | 30 ++++++++++++++++++++++++++++++ > 1 file changed, 30 insertions(+) > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > index f761d9306028..bc2246f27e0d 100644 > --- a/drivers/gpu/drm/drm_encoder.c > +++ b/drivers/gpu/drm/drm_encoder.c > @@ -79,6 +79,34 @@ static void fixup_possible_clones(struct drm_device *dev) > encoder->possible_clones |= drm_encoder_mask(encoder); > } > > +static void validate_possible_clones(struct drm_encoder *encoder) > +{ > + struct drm_device *dev = encoder->dev; > + struct drm_encoder *other; > + u32 encoder_mask = 0; > + > + drm_for_each_encoder(other, dev) { > + encoder_mask |= drm_encoder_mask(other); > + > + WARN(!(encoder->possible_clones & drm_encoder_mask(other)) != > + !(other->possible_clones & drm_encoder_mask(encoder)), > + "possible_clones mismatch: " > + "[ENCODER:%d:%s] mask=0x%x possible_clones=0x%x vs. " > + "[ENCODER:%d:%s] mask=0x%x possible_clones=0x%x\n", > + encoder->base.id, encoder->name, > + drm_encoder_mask(encoder), encoder->possible_clones, > + other->base.id, other->name, > + drm_encoder_mask(other), other->possible_clones); > + } > + > + WARN((encoder->possible_clones & drm_encoder_mask(encoder)) == 0 || > + (encoder->possible_clones & ~encoder_mask) != 0, > + "Bogus possible_clones: " > + "[ENCODER:%d:%s] possible_clones=0x%x (full encoder mask=0x%x)\n", > + encoder->base.id, encoder->name, > + encoder->possible_clones, encoder_mask); > +} > + > int drm_encoder_register_all(struct drm_device *dev) > { > struct drm_encoder *encoder; > @@ -87,6 +115,8 @@ int drm_encoder_register_all(struct drm_device *dev) > fixup_possible_clones(dev); > > drm_for_each_encoder(encoder, dev) { > + validate_possible_clones(encoder); > + > if (encoder->funcs->late_register) > ret = encoder->funcs->late_register(encoder); > if (ret) > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer
Attachment:
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx