On Mon, Mar 06, 2017 at 04:41:05PM +0200, Ville Syrjälä wrote:
> On Mon, Mar 06, 2017 at 04:31:28PM +0200, Jani Nikula wrote:
> > The hook names reflect more the phase in the mode set sequence the hooks
> > are called in than what they actually do in terms of the specific
> > encoder. Stick to that scheme, and rename intel_dsi_pre_disable to
> > intel_dsi_disable. Unify the comments around this while at it. No
> > functional changes.
> >
> > Cc: Madhav Chauhan <madhav.chauhan@xxxxxxxxx>
> > Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> > Cc: Bob Paauwe <bob.j.paauwe@xxxxxxxxx>
> > Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_dsi.c | 21 ++++++++++++---------
> > 1 file changed, 12 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> > index 189b91478f8e..ebe1f55b20d6 100644
> > --- a/drivers/gpu/drm/i915/intel_dsi.c
> > +++ b/drivers/gpu/drm/i915/intel_dsi.c
> > @@ -840,21 +840,24 @@ static void intel_dsi_pre_enable(struct intel_encoder *encoder,
> > intel_dsi_vbt_exec_sequence(intel_dsi, MIPI_SEQ_BACKLIGHT_ON);
> > }
> >
> > +/*
> > + * For DSI port enable has to be done before pipe and plane enable, so port
> > + * enable is done in pre_enable phase unlike other encoders.
>
> The "unlike other encoders" part is nonsense.
We might want to go one step further even and make the hook optional and
remove these few lines here ... Probably not worth it though.
-Daniel
>
> > + */
> > static void intel_dsi_enable_nop(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config,
> > struct drm_connector_state *conn_state)
> > {
> > DRM_DEBUG_KMS("\n");
> > -
> > - /* for DSI port enable has to be done before pipe
> > - * and plane enable, so port enable is done in
> > - * pre_enable phase itself unlike other encoders
> > - */
> > }
> >
> > -static void intel_dsi_pre_disable(struct intel_encoder *encoder,
> > - struct intel_crtc_state *old_crtc_state,
> > - struct drm_connector_state *old_conn_state)
> > +/*
> > + * For DSI port disable has to be done after pipe and plane disable, so port
> > + * disable is done in post_disable phase unlike other encoders.
> > + */
> > +static void intel_dsi_disable(struct intel_encoder *encoder,
> > + struct intel_crtc_state *old_crtc_state,
> > + struct drm_connector_state *old_conn_state)
> > {
> > struct drm_device *dev = encoder->base.dev;
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > @@ -1730,7 +1733,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
> > intel_encoder->compute_config = intel_dsi_compute_config;
> > intel_encoder->pre_enable = intel_dsi_pre_enable;
> > intel_encoder->enable = intel_dsi_enable_nop;
> > - intel_encoder->disable = intel_dsi_pre_disable;
> > + intel_encoder->disable = intel_dsi_disable;
> > intel_encoder->post_disable = intel_dsi_post_disable;
> > intel_encoder->get_hw_state = intel_dsi_get_hw_state;
> > intel_encoder->get_config = intel_dsi_get_config;
> > --
> > 2.1.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
