diff mbox series

[v2,08/16] drm/exynos: add host_ops callback for platform drivers

Message ID 20200911135413.3654800-9-m.tretter@pengutronix.de (mailing list archive)
State Not Applicable
Headers show
Series drm/exynos: Convert driver to drm bridge | expand

Commit Message

Michael Tretter Sept. 11, 2020, 1:54 p.m. UTC
Platform drivers need to be aware if a mipi dsi device attaches or
detaches. Add host_ops to the driver_data for attach and detach
callbacks and move the i80 mode selection and the hotplug handling into
the callback, because these depend on the drm driver.

Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
---
v2:
- new patch
---
 drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++-----
 1 file changed, 53 insertions(+), 11 deletions(-)

Comments

Andrzej Hajda Sept. 15, 2020, 5:07 p.m. UTC | #1
W dniu 11.09.2020 o 15:54, Michael Tretter pisze:
> Platform drivers need to be aware if a mipi dsi device attaches or
> detaches. Add host_ops to the driver_data for attach and detach
> callbacks and move the i80 mode selection and the hotplug handling into
> the callback, because these depend on the drm driver.
>
> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> ---
> v2:
> - new patch
> ---
>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++-----
>   1 file changed, 53 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index 1a15ae71205d..684a2fbef08a 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -239,6 +239,12 @@ struct exynos_dsi_transfer {
>   #define DSIM_STATE_CMD_LPM		BIT(2)
>   #define DSIM_STATE_VIDOUT_AVAILABLE	BIT(3)
>   
> +struct exynos_dsi;
> +struct exynos_dsi_host_ops {
> +	int (*attach)(struct device *dev, struct mipi_dsi_device *device);
> +	int (*detach)(struct device *dev, struct mipi_dsi_device *device);
> +};
> +
>   enum exynos_reg_offset {
>   	EXYNOS_REG_OFS,
>   	EXYNOS5433_REG_OFS
> @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data {
>   	unsigned int wait_for_reset;
>   	unsigned int num_bits_resol;
>   	const unsigned int *reg_values;
> +	const struct exynos_dsi_host_ops *host_ops;
>   };
>   
>   struct exynos_dsi {
> @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = {
>   	[PHYTIMING_HS_TRAIL] = 0x0c,
>   };
>   
> +static int __exynos_dsi_host_attach(struct device *dev,
> +				    struct mipi_dsi_device *device)
> +{
> +	struct exynos_dsi *dsi = dev_get_drvdata(dev);
> +	struct drm_device *drm = dsi->encoder.dev;


As I wrote yesterday drm device was guaranteed to be present here only 
if mipi dsi host registration was performed in component bind. In case 
it is performed in probe it will be always NULL, and the code does not 
make sense.


Regards

Andrzej



> +	struct exynos_drm_crtc *crtc;
> +
> +	mutex_lock(&drm->mode_config.mutex);
> +	crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD);
> +	crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO);
> +	mutex_unlock(&drm->mode_config.mutex);
> +
> +	if (drm->mode_config.poll_enabled)
> +		drm_kms_helper_hotplug_event(drm);
> +
> +	return 0;
> +}
> +
> +static int __exynos_dsi_host_detach(struct device *dev,
> +				     struct mipi_dsi_device *device)
> +{
> +	struct exynos_dsi *dsi = dev_get_drvdata(dev);
> +	struct drm_device *drm = dsi->encoder.dev;
> +
> +	if (drm->mode_config.poll_enabled)
> +		drm_kms_helper_hotplug_event(drm);
> +
> +	return 0;
> +}
> +
> +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = {
> +	.attach = __exynos_dsi_host_attach,
> +	.detach = __exynos_dsi_host_detach,
> +};
> +
>   static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
>   	.reg_ofs = EXYNOS_REG_OFS,
>   	.plltmr_reg = 0x50,
> @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
>   	.wait_for_reset = 1,
>   	.num_bits_resol = 11,
>   	.reg_values = reg_values,
> +	.host_ops = &exynos_dsi_host_ops,
>   };
>   
>   static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
> @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
>   	.wait_for_reset = 1,
>   	.num_bits_resol = 11,
>   	.reg_values = reg_values,
> +	.host_ops = &exynos_dsi_host_ops,
>   };
>   
>   static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
> @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
>   	.wait_for_reset = 1,
>   	.num_bits_resol = 11,
>   	.reg_values = reg_values,
> +	.host_ops = &exynos_dsi_host_ops,
>   };
>   
>   static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
> @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
>   	.wait_for_reset = 0,
>   	.num_bits_resol = 12,
>   	.reg_values = exynos5433_reg_values,
> +	.host_ops = &exynos_dsi_host_ops,
>   };
>   
>   static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
> @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
>   	.wait_for_reset = 1,
>   	.num_bits_resol = 12,
>   	.reg_values = exynos5422_reg_values,
> +	.host_ops = &exynos_dsi_host_ops,
>   };
>   
>   static const struct of_device_id exynos_dsi_of_match[] = {
> @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>   				  struct mipi_dsi_device *device)
>   {
>   	struct exynos_dsi *dsi = host_to_dsi(host);
> +	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
>   	struct drm_encoder *encoder = &dsi->encoder;
> -	struct drm_device *drm = encoder->dev;
>   	struct drm_bridge *out_bridge;
>   
>   	out_bridge  = of_drm_find_bridge(device->dev.of_node);
> @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>   			return ret;
>   	}
>   
> -	mutex_lock(&drm->mode_config.mutex);
> -
>   	dsi->lanes = device->lanes;
>   	dsi->format = device->format;
>   	dsi->mode_flags = device->mode_flags;
> -	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
> -			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
>   
> -	mutex_unlock(&drm->mode_config.mutex);
> -
> -	if (drm->mode_config.poll_enabled)
> -		drm_kms_helper_hotplug_event(drm);
> +	if (ops && ops->attach)
> +		ops->attach(dsi->dsi_host.dev, device);
>   
>   	return 0;
>   }
> @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>   				  struct mipi_dsi_device *device)
>   {
>   	struct exynos_dsi *dsi = host_to_dsi(host);
> +	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
>   	struct drm_device *drm = dsi->encoder.dev;
>   
>   	if (dsi->panel) {
> @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>   		INIT_LIST_HEAD(&dsi->bridge_chain);
>   	}
>   
> -	if (drm->mode_config.poll_enabled)
> -		drm_kms_helper_hotplug_event(drm);
> +	if (ops && ops->detach)
> +		ops->detach(dsi->dsi_host.dev, device);
>   
>   	exynos_dsi_unregister_te_irq(dsi);
>
Michael Tretter Sept. 15, 2020, 6:02 p.m. UTC | #2
On Tue, 15 Sep 2020 19:07:59 +0200, Andrzej Hajda wrote:
> 
> W dniu 11.09.2020 o 15:54, Michael Tretter pisze:
> > Platform drivers need to be aware if a mipi dsi device attaches or
> > detaches. Add host_ops to the driver_data for attach and detach
> > callbacks and move the i80 mode selection and the hotplug handling into
> > the callback, because these depend on the drm driver.
> >
> > Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
> > ---
> > v2:
> > - new patch
> > ---
> >   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++-----
> >   1 file changed, 53 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > index 1a15ae71205d..684a2fbef08a 100644
> > --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> > @@ -239,6 +239,12 @@ struct exynos_dsi_transfer {
> >   #define DSIM_STATE_CMD_LPM		BIT(2)
> >   #define DSIM_STATE_VIDOUT_AVAILABLE	BIT(3)
> >   
> > +struct exynos_dsi;
> > +struct exynos_dsi_host_ops {
> > +	int (*attach)(struct device *dev, struct mipi_dsi_device *device);
> > +	int (*detach)(struct device *dev, struct mipi_dsi_device *device);
> > +};
> > +
> >   enum exynos_reg_offset {
> >   	EXYNOS_REG_OFS,
> >   	EXYNOS5433_REG_OFS
> > @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data {
> >   	unsigned int wait_for_reset;
> >   	unsigned int num_bits_resol;
> >   	const unsigned int *reg_values;
> > +	const struct exynos_dsi_host_ops *host_ops;
> >   };
> >   
> >   struct exynos_dsi {
> > @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = {
> >   	[PHYTIMING_HS_TRAIL] = 0x0c,
> >   };
> >   
> > +static int __exynos_dsi_host_attach(struct device *dev,
> > +				    struct mipi_dsi_device *device)
> > +{
> > +	struct exynos_dsi *dsi = dev_get_drvdata(dev);
> > +	struct drm_device *drm = dsi->encoder.dev;
> 
> 
> As I wrote yesterday drm device was guaranteed to be present here only 
> if mipi dsi host registration was performed in component bind. In case 
> it is performed in probe it will be always NULL, and the code does not 
> make sense.

Correct, but if the driver is used as a drm bridge, there won't be an encoder
until bridge_attach. Mipi dsi devices defer their probe until the mipi dsi
host is available. If I move the mipi dsi host registration into
bridge_attach, the driver does not know if the next device is another bridge
or a panel during bridge_attach, but the driver cannot successfully return
from bridge_attach, because then the drm driver expects a connector.

I guess that the encoder should be initialized before registering the mipi dsi
host during probe instead of bind. The probe won't be rolled back on
PROBE_DEFER during bind and the encoder will be available in host_attach.
When splitting the driver into the exynos platform specific code and the more
generic code, there won't be an encoder during host_attach in the generic
code, but the host_ops callback could (and will) use the platform specific
encoder, which is available before bridge_attach.

Does this make sense to you?

Michael

> 
> 
> Regards
> 
> Andrzej
> 
> 
> 
> > +	struct exynos_drm_crtc *crtc;
> > +
> > +	mutex_lock(&drm->mode_config.mutex);
> > +	crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD);
> > +	crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO);
> > +	mutex_unlock(&drm->mode_config.mutex);
> > +
> > +	if (drm->mode_config.poll_enabled)
> > +		drm_kms_helper_hotplug_event(drm);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __exynos_dsi_host_detach(struct device *dev,
> > +				     struct mipi_dsi_device *device)
> > +{
> > +	struct exynos_dsi *dsi = dev_get_drvdata(dev);
> > +	struct drm_device *drm = dsi->encoder.dev;
> > +
> > +	if (drm->mode_config.poll_enabled)
> > +		drm_kms_helper_hotplug_event(drm);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = {
> > +	.attach = __exynos_dsi_host_attach,
> > +	.detach = __exynos_dsi_host_detach,
> > +};
> > +
> >   static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
> >   	.reg_ofs = EXYNOS_REG_OFS,
> >   	.plltmr_reg = 0x50,
> > @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
> >   	.wait_for_reset = 1,
> >   	.num_bits_resol = 11,
> >   	.reg_values = reg_values,
> > +	.host_ops = &exynos_dsi_host_ops,
> >   };
> >   
> >   static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
> > @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
> >   	.wait_for_reset = 1,
> >   	.num_bits_resol = 11,
> >   	.reg_values = reg_values,
> > +	.host_ops = &exynos_dsi_host_ops,
> >   };
> >   
> >   static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
> > @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
> >   	.wait_for_reset = 1,
> >   	.num_bits_resol = 11,
> >   	.reg_values = reg_values,
> > +	.host_ops = &exynos_dsi_host_ops,
> >   };
> >   
> >   static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
> > @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
> >   	.wait_for_reset = 0,
> >   	.num_bits_resol = 12,
> >   	.reg_values = exynos5433_reg_values,
> > +	.host_ops = &exynos_dsi_host_ops,
> >   };
> >   
> >   static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
> > @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
> >   	.wait_for_reset = 1,
> >   	.num_bits_resol = 12,
> >   	.reg_values = exynos5422_reg_values,
> > +	.host_ops = &exynos_dsi_host_ops,
> >   };
> >   
> >   static const struct of_device_id exynos_dsi_of_match[] = {
> > @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> >   				  struct mipi_dsi_device *device)
> >   {
> >   	struct exynos_dsi *dsi = host_to_dsi(host);
> > +	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
> >   	struct drm_encoder *encoder = &dsi->encoder;
> > -	struct drm_device *drm = encoder->dev;
> >   	struct drm_bridge *out_bridge;
> >   
> >   	out_bridge  = of_drm_find_bridge(device->dev.of_node);
> > @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
> >   			return ret;
> >   	}
> >   
> > -	mutex_lock(&drm->mode_config.mutex);
> > -
> >   	dsi->lanes = device->lanes;
> >   	dsi->format = device->format;
> >   	dsi->mode_flags = device->mode_flags;
> > -	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
> > -			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
> >   
> > -	mutex_unlock(&drm->mode_config.mutex);
> > -
> > -	if (drm->mode_config.poll_enabled)
> > -		drm_kms_helper_hotplug_event(drm);
> > +	if (ops && ops->attach)
> > +		ops->attach(dsi->dsi_host.dev, device);
> >   
> >   	return 0;
> >   }
> > @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
> >   				  struct mipi_dsi_device *device)
> >   {
> >   	struct exynos_dsi *dsi = host_to_dsi(host);
> > +	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
> >   	struct drm_device *drm = dsi->encoder.dev;
> >   
> >   	if (dsi->panel) {
> > @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
> >   		INIT_LIST_HEAD(&dsi->bridge_chain);
> >   	}
> >   
> > -	if (drm->mode_config.poll_enabled)
> > -		drm_kms_helper_hotplug_event(drm);
> > +	if (ops && ops->detach)
> > +		ops->detach(dsi->dsi_host.dev, device);
> >   
> >   	exynos_dsi_unregister_te_irq(dsi);
> >   
>
Andrzej Hajda Sept. 16, 2020, 10:01 p.m. UTC | #3
W dniu 15.09.2020 o 20:02, Michael Tretter pisze:
> On Tue, 15 Sep 2020 19:07:59 +0200, Andrzej Hajda wrote:
>> W dniu 11.09.2020 o 15:54, Michael Tretter pisze:
>>> Platform drivers need to be aware if a mipi dsi device attaches or
>>> detaches. Add host_ops to the driver_data for attach and detach
>>> callbacks and move the i80 mode selection and the hotplug handling into
>>> the callback, because these depend on the drm driver.
>>>
>>> Signed-off-by: Michael Tretter <m.tretter@pengutronix.de>
>>> ---
>>> v2:
>>> - new patch
>>> ---
>>>    drivers/gpu/drm/exynos/exynos_drm_dsi.c | 64 ++++++++++++++++++++-----
>>>    1 file changed, 53 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index 1a15ae71205d..684a2fbef08a 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -239,6 +239,12 @@ struct exynos_dsi_transfer {
>>>    #define DSIM_STATE_CMD_LPM		BIT(2)
>>>    #define DSIM_STATE_VIDOUT_AVAILABLE	BIT(3)
>>>    
>>> +struct exynos_dsi;
>>> +struct exynos_dsi_host_ops {
>>> +	int (*attach)(struct device *dev, struct mipi_dsi_device *device);
>>> +	int (*detach)(struct device *dev, struct mipi_dsi_device *device);
>>> +};
>>> +
>>>    enum exynos_reg_offset {
>>>    	EXYNOS_REG_OFS,
>>>    	EXYNOS5433_REG_OFS
>>> @@ -254,6 +260,7 @@ struct exynos_dsi_driver_data {
>>>    	unsigned int wait_for_reset;
>>>    	unsigned int num_bits_resol;
>>>    	const unsigned int *reg_values;
>>> +	const struct exynos_dsi_host_ops *host_ops;
>>>    };
>>>    
>>>    struct exynos_dsi {
>>> @@ -467,6 +474,41 @@ static const unsigned int exynos5433_reg_values[] = {
>>>    	[PHYTIMING_HS_TRAIL] = 0x0c,
>>>    };
>>>    
>>> +static int __exynos_dsi_host_attach(struct device *dev,
>>> +				    struct mipi_dsi_device *device)
>>> +{
>>> +	struct exynos_dsi *dsi = dev_get_drvdata(dev);
>>> +	struct drm_device *drm = dsi->encoder.dev;
>>
>> As I wrote yesterday drm device was guaranteed to be present here only
>> if mipi dsi host registration was performed in component bind. In case
>> it is performed in probe it will be always NULL, and the code does not
>> make sense.
> Correct, but if the driver is used as a drm bridge, there won't be an encoder
> until bridge_attach. Mipi dsi devices defer their probe until the mipi dsi
> host is available. If I move the mipi dsi host registration into
> bridge_attach, the driver does not know if the next device is another bridge
> or a panel during bridge_attach, but the driver cannot successfully return
> from bridge_attach, because then the drm driver expects a connector.


Hmm, I am not sure why do you think driver expects connector on return 
of briddge_attach.


>
> I guess that the encoder should be initialized before registering the mipi dsi
> host during probe instead of bind.


But you should have already drm dev on encoder init which in probe is 
unavailable.



> The probe won't be rolled back on
> PROBE_DEFER during bind and the encoder will be available in host_attach.
> When splitting the driver into the exynos platform specific code and the more
> generic code, there won't be an encoder during host_attach in the generic
> code, but the host_ops callback could (and will) use the platform specific
> encoder, which is available before bridge_attach.
>
> Does this make sense to you?


Nope :) But maybe I am missing sth.

Generally I see two ways (which I have already described in different 
e-mail, in different words):

A. Static - we wait for every part of display stack to be probed, then 
create drm_dev - typical approach, but slow (deferred probe causes late 
drm creation), and racy - only(?) component framework and DSI bus have 
possibility to signal driver unbind, so we can react on it properly.

B. Dynamic - drm framework requires only crtcs and encoders to be 
attached to drm on init, connectors, and hidden parts (drm_bridges, 
drm_panels) can be created/destroyed and attached/detached at any time 
(almost), so lets take advantage of it - create drm_dev ASAP and attach 
other parts when they become available, the only issue is that there is 
no generic way to be notified when given parts becomes available - in 
interesting area only mipi devices have such notifications via attach 
callbacks.


So either we convert exynos_dsi to A either we continue B approach. In 
second case we should assure mipi_dsi host is created if drm_dev is 
available.


Regards

Andrzej



>
> Michael
>
>>
>> Regards
>>
>> Andrzej
>>
>>
>>
>>> +	struct exynos_drm_crtc *crtc;
>>> +
>>> +	mutex_lock(&drm->mode_config.mutex);
>>> +	crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD);
>>> +	crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO);
>>> +	mutex_unlock(&drm->mode_config.mutex);
>>> +
>>> +	if (drm->mode_config.poll_enabled)
>>> +		drm_kms_helper_hotplug_event(drm);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int __exynos_dsi_host_detach(struct device *dev,
>>> +				     struct mipi_dsi_device *device)
>>> +{
>>> +	struct exynos_dsi *dsi = dev_get_drvdata(dev);
>>> +	struct drm_device *drm = dsi->encoder.dev;
>>> +
>>> +	if (drm->mode_config.poll_enabled)
>>> +		drm_kms_helper_hotplug_event(drm);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct exynos_dsi_host_ops exynos_dsi_host_ops = {
>>> +	.attach = __exynos_dsi_host_attach,
>>> +	.detach = __exynos_dsi_host_detach,
>>> +};
>>> +
>>>    static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
>>>    	.reg_ofs = EXYNOS_REG_OFS,
>>>    	.plltmr_reg = 0x50,
>>> @@ -477,6 +519,7 @@ static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
>>>    	.wait_for_reset = 1,
>>>    	.num_bits_resol = 11,
>>>    	.reg_values = reg_values,
>>> +	.host_ops = &exynos_dsi_host_ops,
>>>    };
>>>    
>>>    static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
>>> @@ -489,6 +532,7 @@ static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
>>>    	.wait_for_reset = 1,
>>>    	.num_bits_resol = 11,
>>>    	.reg_values = reg_values,
>>> +	.host_ops = &exynos_dsi_host_ops,
>>>    };
>>>    
>>>    static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
>>> @@ -499,6 +543,7 @@ static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
>>>    	.wait_for_reset = 1,
>>>    	.num_bits_resol = 11,
>>>    	.reg_values = reg_values,
>>> +	.host_ops = &exynos_dsi_host_ops,
>>>    };
>>>    
>>>    static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
>>> @@ -510,6 +555,7 @@ static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
>>>    	.wait_for_reset = 0,
>>>    	.num_bits_resol = 12,
>>>    	.reg_values = exynos5433_reg_values,
>>> +	.host_ops = &exynos_dsi_host_ops,
>>>    };
>>>    
>>>    static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
>>> @@ -521,6 +567,7 @@ static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
>>>    	.wait_for_reset = 1,
>>>    	.num_bits_resol = 12,
>>>    	.reg_values = exynos5422_reg_values,
>>> +	.host_ops = &exynos_dsi_host_ops,
>>>    };
>>>    
>>>    static const struct of_device_id exynos_dsi_of_match[] = {
>>> @@ -1551,8 +1598,8 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>>    				  struct mipi_dsi_device *device)
>>>    {
>>>    	struct exynos_dsi *dsi = host_to_dsi(host);
>>> +	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
>>>    	struct drm_encoder *encoder = &dsi->encoder;
>>> -	struct drm_device *drm = encoder->dev;
>>>    	struct drm_bridge *out_bridge;
>>>    
>>>    	out_bridge  = of_drm_find_bridge(device->dev.of_node);
>>> @@ -1590,18 +1637,12 @@ static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
>>>    			return ret;
>>>    	}
>>>    
>>> -	mutex_lock(&drm->mode_config.mutex);
>>> -
>>>    	dsi->lanes = device->lanes;
>>>    	dsi->format = device->format;
>>>    	dsi->mode_flags = device->mode_flags;
>>> -	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
>>> -			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
>>>    
>>> -	mutex_unlock(&drm->mode_config.mutex);
>>> -
>>> -	if (drm->mode_config.poll_enabled)
>>> -		drm_kms_helper_hotplug_event(drm);
>>> +	if (ops && ops->attach)
>>> +		ops->attach(dsi->dsi_host.dev, device);
>>>    
>>>    	return 0;
>>>    }
>>> @@ -1610,6 +1651,7 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>>>    				  struct mipi_dsi_device *device)
>>>    {
>>>    	struct exynos_dsi *dsi = host_to_dsi(host);
>>> +	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
>>>    	struct drm_device *drm = dsi->encoder.dev;
>>>    
>>>    	if (dsi->panel) {
>>> @@ -1625,8 +1667,8 @@ static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
>>>    		INIT_LIST_HEAD(&dsi->bridge_chain);
>>>    	}
>>>    
>>> -	if (drm->mode_config.poll_enabled)
>>> -		drm_kms_helper_hotplug_event(drm);
>>> +	if (ops && ops->detach)
>>> +		ops->detach(dsi->dsi_host.dev, device);
>>>    
>>>    	exynos_dsi_unregister_te_irq(dsi);
>>>    
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://protect2.fireeye.com/v1/url?k=80c78954-dd15920c-80c6021b-0cc47a31c8b4-c39fad41cb70b194&q=1&e=b51d6682-ba72-48c0-b0c9-013866ba39ab&u=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
index 1a15ae71205d..684a2fbef08a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
@@ -239,6 +239,12 @@  struct exynos_dsi_transfer {
 #define DSIM_STATE_CMD_LPM		BIT(2)
 #define DSIM_STATE_VIDOUT_AVAILABLE	BIT(3)
 
+struct exynos_dsi;
+struct exynos_dsi_host_ops {
+	int (*attach)(struct device *dev, struct mipi_dsi_device *device);
+	int (*detach)(struct device *dev, struct mipi_dsi_device *device);
+};
+
 enum exynos_reg_offset {
 	EXYNOS_REG_OFS,
 	EXYNOS5433_REG_OFS
@@ -254,6 +260,7 @@  struct exynos_dsi_driver_data {
 	unsigned int wait_for_reset;
 	unsigned int num_bits_resol;
 	const unsigned int *reg_values;
+	const struct exynos_dsi_host_ops *host_ops;
 };
 
 struct exynos_dsi {
@@ -467,6 +474,41 @@  static const unsigned int exynos5433_reg_values[] = {
 	[PHYTIMING_HS_TRAIL] = 0x0c,
 };
 
+static int __exynos_dsi_host_attach(struct device *dev,
+				    struct mipi_dsi_device *device)
+{
+	struct exynos_dsi *dsi = dev_get_drvdata(dev);
+	struct drm_device *drm = dsi->encoder.dev;
+	struct exynos_drm_crtc *crtc;
+
+	mutex_lock(&drm->mode_config.mutex);
+	crtc = exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD);
+	crtc->i80_mode = !(device->mode_flags & MIPI_DSI_MODE_VIDEO);
+	mutex_unlock(&drm->mode_config.mutex);
+
+	if (drm->mode_config.poll_enabled)
+		drm_kms_helper_hotplug_event(drm);
+
+	return 0;
+}
+
+static int __exynos_dsi_host_detach(struct device *dev,
+				     struct mipi_dsi_device *device)
+{
+	struct exynos_dsi *dsi = dev_get_drvdata(dev);
+	struct drm_device *drm = dsi->encoder.dev;
+
+	if (drm->mode_config.poll_enabled)
+		drm_kms_helper_hotplug_event(drm);
+
+	return 0;
+}
+
+static const struct exynos_dsi_host_ops exynos_dsi_host_ops = {
+	.attach = __exynos_dsi_host_attach,
+	.detach = __exynos_dsi_host_detach,
+};
+
 static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
 	.reg_ofs = EXYNOS_REG_OFS,
 	.plltmr_reg = 0x50,
@@ -477,6 +519,7 @@  static const struct exynos_dsi_driver_data exynos3_dsi_driver_data = {
 	.wait_for_reset = 1,
 	.num_bits_resol = 11,
 	.reg_values = reg_values,
+	.host_ops = &exynos_dsi_host_ops,
 };
 
 static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
@@ -489,6 +532,7 @@  static const struct exynos_dsi_driver_data exynos4_dsi_driver_data = {
 	.wait_for_reset = 1,
 	.num_bits_resol = 11,
 	.reg_values = reg_values,
+	.host_ops = &exynos_dsi_host_ops,
 };
 
 static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
@@ -499,6 +543,7 @@  static const struct exynos_dsi_driver_data exynos5_dsi_driver_data = {
 	.wait_for_reset = 1,
 	.num_bits_resol = 11,
 	.reg_values = reg_values,
+	.host_ops = &exynos_dsi_host_ops,
 };
 
 static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
@@ -510,6 +555,7 @@  static const struct exynos_dsi_driver_data exynos5433_dsi_driver_data = {
 	.wait_for_reset = 0,
 	.num_bits_resol = 12,
 	.reg_values = exynos5433_reg_values,
+	.host_ops = &exynos_dsi_host_ops,
 };
 
 static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
@@ -521,6 +567,7 @@  static const struct exynos_dsi_driver_data exynos5422_dsi_driver_data = {
 	.wait_for_reset = 1,
 	.num_bits_resol = 12,
 	.reg_values = exynos5422_reg_values,
+	.host_ops = &exynos_dsi_host_ops,
 };
 
 static const struct of_device_id exynos_dsi_of_match[] = {
@@ -1551,8 +1598,8 @@  static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 				  struct mipi_dsi_device *device)
 {
 	struct exynos_dsi *dsi = host_to_dsi(host);
+	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
 	struct drm_encoder *encoder = &dsi->encoder;
-	struct drm_device *drm = encoder->dev;
 	struct drm_bridge *out_bridge;
 
 	out_bridge  = of_drm_find_bridge(device->dev.of_node);
@@ -1590,18 +1637,12 @@  static int exynos_dsi_host_attach(struct mipi_dsi_host *host,
 			return ret;
 	}
 
-	mutex_lock(&drm->mode_config.mutex);
-
 	dsi->lanes = device->lanes;
 	dsi->format = device->format;
 	dsi->mode_flags = device->mode_flags;
-	exynos_drm_crtc_get_by_type(drm, EXYNOS_DISPLAY_TYPE_LCD)->i80_mode =
-			!(dsi->mode_flags & MIPI_DSI_MODE_VIDEO);
 
-	mutex_unlock(&drm->mode_config.mutex);
-
-	if (drm->mode_config.poll_enabled)
-		drm_kms_helper_hotplug_event(drm);
+	if (ops && ops->attach)
+		ops->attach(dsi->dsi_host.dev, device);
 
 	return 0;
 }
@@ -1610,6 +1651,7 @@  static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
 				  struct mipi_dsi_device *device)
 {
 	struct exynos_dsi *dsi = host_to_dsi(host);
+	const struct exynos_dsi_host_ops *ops = dsi->driver_data->host_ops;
 	struct drm_device *drm = dsi->encoder.dev;
 
 	if (dsi->panel) {
@@ -1625,8 +1667,8 @@  static int exynos_dsi_host_detach(struct mipi_dsi_host *host,
 		INIT_LIST_HEAD(&dsi->bridge_chain);
 	}
 
-	if (drm->mode_config.poll_enabled)
-		drm_kms_helper_hotplug_event(drm);
+	if (ops && ops->detach)
+		ops->detach(dsi->dsi_host.dev, device);
 
 	exynos_dsi_unregister_te_irq(dsi);