[PATCH 2/4] drm: exynos: Remove dummy encoder get_crtc operation implementation

[Laurent Pinchart]

Hi Inki,

On Thursday 17 May 2012 17:21:46 Inki Dae wrote:
> On Thursday, May 17, 2012 12:09 AM Laurent Pinchart wrote:
> > 
> > The encoder get_crtc operation is called to retrieve a pointer to the
> > CRTC the encoder is currenctly connected to, right after setting the
> > encoder::crtc field to the new CRTC. The implementation of this
> > operation returns the pointer to the new CRTC, which is then pointlessly
> > compared to itself.
> > 
> > As the operation is not mandatory, don't implement it.
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> > 
> >  drivers/gpu/drm/exynos/exynos_drm_encoder.c |    7 -------
> >  1 files changed, 0 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> > b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> > index 6e9ac7b..23d5ad3 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> > @@ -172,19 +172,12 @@ static void exynos_drm_encoder_commit(struct
> > drm_encoder *encoder)
> > 
> >  		manager_ops->commit(manager->dev);
> >  
> >  }
> > 
> > -static struct drm_crtc *
> > -exynos_drm_encoder_get_crtc(struct drm_encoder *encoder)
> > -{
> > -	return encoder->crtc;
> > -}
> > -
> > 
> >  static struct drm_encoder_helper_funcs exynos_encoder_helper_funcs = {
> >  
> >  	.dpms		= exynos_drm_encoder_dpms,
> >  	.mode_fixup	= exynos_drm_encoder_mode_fixup,
> >  	.mode_set	= exynos_drm_encoder_mode_set,
> >  	.prepare	= exynos_drm_encoder_prepare,
> >  	.commit		= exynos_drm_encoder_commit,
> > 
> > -	.get_crtc	= exynos_drm_encoder_get_crtc,
> > 
> >  };
> 
> We need get_crtc callback to disable encoder whose CRTC is about to change.

The get_crtc operation is called in a single location, in drm_crtc_helper.c:

                /* Disable encoders whose CRTC is about to change */
                if (encoder_funcs->get_crtc &&
                    encoder->crtc != (*encoder_funcs->get_crtc)(encoder))
                        drm_encoder_disable(encoder);

Your implementation returns encoder->crtc, so the second part of the test is 
always false. drm_encoder_disable() is never called here, the get_crtc 
implementation is either unneeded or incorrect.

-- 
Regards,

Laurent Pinchart