Re: [PATCH v3 1/3] drm/bridge: introduce bridge detaching mechanism

[Archit Taneja <architt@codeaurora.org>]

On 08/25/2016 05:46 PM, Daniel Vetter wrote:
> On Thu, Aug 25, 2016 at 11:04:32AM +0200, Andrea Merello wrote:
>> Up to now, once a bridge has been attached to a DRM device, it cannot
>> be undone.
>>
>> In particular you couldn't rmmod/insmod a DRM driver that uses a bridge,
>> because the bridge would remain bound to the first (dead) driver instance.
>>
>> This patch fixes this by introducing drm_encoder_detach() and a ->detach
>> callback in drm_bridge_funcs for the bridge to be notified about detaches.
>>
>> It's DRM/KMS driver responsibility to call drm_encoder_detach().
>>
>> While adding the bridge detach callback, with its kerneldoc, I also added
>> kerneldoc for attach callback.
>>
>> Few other kerneldocs fixes around there are included.
>>
>> Suggested-by: Daniel Vetter <daniel@ffwll.ch>
>> Suggested-by: Lucas Stach <l.stach@pengutronix.de>
>> Signed-off-by: Andrea Merello <andrea.merello@gmail.com>
>> Cc: Archit Taneja <architt@codeaurora.org>
>> Cc: David Airlie <airlied@linux.ie>
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Lucas Stach <l.stach@pengutronix.de>
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>
> As discussed on the last round, I'll let Archit apply all 3 to drm-misc,
> so he can check them out too.

Pushed to drm-misc.

Thanks,
Archit

> -Daniel
>> ---
>>   drivers/gpu/drm/drm_bridge.c | 29 +++++++++++++++++++++++++++--
>>   include/drm/drm_crtc.h       | 26 +++++++++++++++++++++++++-
>>   2 files changed, 52 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
>> index 2555430..4840466 100644
>> --- a/drivers/gpu/drm/drm_bridge.c
>> +++ b/drivers/gpu/drm/drm_bridge.c
>> @@ -98,11 +98,11 @@ EXPORT_SYMBOL(drm_bridge_remove);
>>    * @dev: DRM device
>>    * @bridge: bridge control structure
>>    *
>> - * called by a kms driver to link one of our encoder/bridge to the given
>> + * Called by a kms driver to link one of our encoder/bridge to the given
>>    * bridge.
>>    *
>>    * Note that setting up links between the bridge and our encoder/bridge
>> - * objects needs to be handled by the kms driver itself
>> + * objects needs to be handled by the kms driver itself.
>>    *
>>    * RETURNS:
>>    * Zero on success, error code on failure
>> @@ -125,6 +125,31 @@ int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge)
>>   EXPORT_SYMBOL(drm_bridge_attach);
>>
>>   /**
>> + * drm_bridge_detach - deassociate given bridge from its DRM device
>> + *
>> + * @bridge: bridge control structure
>> + *
>> + * Called by a kms driver to unlink the given bridge from its DRM device.
>> + *
>> + * Note that tearing down links between the bridge and our encoder/bridge
>> + * objects needs to be handled by the kms driver itself.
>> + */
>> +void drm_bridge_detach(struct drm_bridge *bridge)
>> +{
>> +	if (WARN_ON(!bridge))
>> +		return;
>> +
>> +	if (WARN_ON(!bridge->dev))
>> +		return;
>> +
>> +	if (bridge->funcs->detach)
>> +		bridge->funcs->detach(bridge);
>> +
>> +	bridge->dev = NULL;
>> +}
>> +EXPORT_SYMBOL(drm_bridge_detach);
>> +
>> +/**
>>    * DOC: bridge callbacks
>>    *
>>    * The &drm_bridge_funcs ops are populated by the bridge driver. The DRM
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 3fa0275..bb214a1 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -1118,12 +1118,33 @@ struct drm_plane {
>>
>>   /**
>>    * struct drm_bridge_funcs - drm_bridge control functions
>> - * @attach: Called during drm_bridge_attach
>>    */
>>   struct drm_bridge_funcs {
>> +	/**
>> +	 * @attach:
>> +	 *
>> +	 * This callback is invoked whenever our bridge is being attached to a
>> +	 * &drm_encoder.
>> +	 *
>> +	 * The attach callback is optional.
>> +	 *
>> +	 * RETURNS:
>> +	 *
>> +	 * Zero on success, error code on failure.
>> +	 */
>>   	int (*attach)(struct drm_bridge *bridge);
>>
>>   	/**
>> +	 * @detach:
>> +	 *
>> +	 * This callback is invoked whenever our bridge is being detached from a
>> +	 * &drm_encoder.
>> +	 *
>> +	 * The detach callback is optional.
>> +	 */
>> +	void (*detach)(struct drm_bridge *bridge);
>> +
>> +	/**
>>   	 * @mode_fixup:
>>   	 *
>>   	 * This callback is used to validate and adjust a mode. The paramater
>> @@ -1137,6 +1158,8 @@ struct drm_bridge_funcs {
>>   	 * this function passes all other callbacks must succeed for this
>>   	 * configuration.
>>   	 *
>> +	 * The mode_fixup callback is optional.
>> +	 *
>>   	 * NOTE:
>>   	 *
>>   	 * This function is called in the check phase of atomic modesets, which
>> @@ -2426,6 +2449,7 @@ extern int drm_bridge_add(struct drm_bridge *bridge);
>>   extern void drm_bridge_remove(struct drm_bridge *bridge);
>>   extern struct drm_bridge *of_drm_find_bridge(struct device_node *np);
>>   extern int drm_bridge_attach(struct drm_device *dev, struct drm_bridge *bridge);
>> +extern void drm_bridge_detach(struct drm_bridge *bridge);
>>
>>   bool drm_bridge_mode_fixup(struct drm_bridge *bridge,
>>   			const struct drm_display_mode *mode,
>> --
>> 2.7.4
>>
>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel