Re: [PATCH 1/3] drm/i915: Split encoder->hot_plug() into pre and post variants

[Manasi Navare <manasi.d.navare@intel.com>]

On Fri, Dec 22, 2017 at 08:28:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> During hpd processing we may want to do things both before and
> after the display detection. To that end split the encoder->hot_plug()
> hooks.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h     | 3 ++-
>  drivers/gpu/drm/i915/intel_hotplug.c | 6 ++++--
>  drivers/gpu/drm/i915/intel_sdvo.c    | 2 +-
>  3 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 30f791f89d64..4a7e603ccc38 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -211,7 +211,8 @@ struct intel_encoder {
>  	enum intel_output_type type;
>  	enum port port;
>  	unsigned int cloneable;
> -	void (*hot_plug)(struct intel_encoder *);
> +	void (*pre_hotplug)(struct intel_encoder *encoder);
> +	void (*post_hotplug)(struct intel_encoder *encoder);

Splitting into pre and post detect for the hotplug makes sense.
Although to be more intuitive of the order of things that happen on hotplug
the names could be hotplug_pre_detect() and hotplug_post_detect() or something
on those lines relative to the detect() call.
 Looks good other than that.

Manasi

>  	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
>  						      struct intel_crtc_state *,
>  						      struct drm_connector_state *);
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 875d5d218d5c..91abca3ae11a 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -370,10 +370,12 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
>  			DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
>  				      connector->name, intel_encoder->hpd_pin);
> -			if (intel_encoder->hot_plug)
> -				intel_encoder->hot_plug(intel_encoder);
> +			if (intel_encoder->pre_hotplug)
> +				intel_encoder->pre_hotplug(intel_encoder);
>  			if (intel_hpd_irq_event(dev, connector))
>  				changed = true;
> +			if (intel_encoder->post_hotplug)
> +				intel_encoder->post_hotplug(intel_encoder);
>  		}
>  	}
>  	drm_connector_list_iter_end(&conn_iter);
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 2b8764897d68..6de90a1fc553 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -2496,7 +2496,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  		/* Some SDVO devices have one-shot hotplug interrupts.
>  		 * Ensure that they get re-enabled when an interrupt happens.
>  		 */
> -		intel_encoder->hot_plug = intel_sdvo_enable_hotplug;
> +		intel_encoder->pre_hotplug = intel_sdvo_enable_hotplug;
>  		intel_sdvo_enable_hotplug(intel_encoder);
>  	} else {
>  		intel_connector->polled = DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT;
> -- 
> 2.13.6
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx