Re: [PATCH 5/7] drm/i915/dsi: rename intel_dsi_pre_disable to intel_dsi_disable

[Jani Nikula <jani.nikula@intel.com>]

On Mon, 06 Mar 2017, Daniel Vetter <daniel@ffwll.ch> wrote:
> 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@intel.com>
>> > Cc: Hans de Goede <hdegoede@redhat.com>
>> > Cc: Bob Paauwe <bob.j.paauwe@intel.com>
>> > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> > ---
>> >  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.

I looked into it before, and decided it's better to keep intel_display.c
nice and clean, as most if not all other encoders provide the hook.

BR,
Jani.


> -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@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> 
>> -- 
>> Ville Syrjälä
>> Intel OTC
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx