[PATCH 11/11] drm/exynos: remove struct exynos_drm_encoder layer

Inki Dae inki.dae at samsung.com
Tue Aug 4 18:31:22 PDT 2015


On 2015년 08월 04일 23:47, Gustavo Padovan wrote:
> Hi Inki,
> 
> 2015-08-04 Inki Dae <inki.dae at samsung.com>:
> 
>> On 2015년 08월 04일 04:09, Gustavo Padovan wrote:
>>> From: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>>
>>> struct exynos_drm_encoder was justing wrapping struct drm_encoder, it had
>>> only a drm_encoder member and the internal exynos_drm_encoders ops that
>>> was directly mapped to the drm_encoder helper funcs.
>>>
>>> So now exynos DRM uses struct drm_encoder directly, this removes
>>> completely the struct exynos_drm_encoder.
>>>
>>> Signed-off-by: Gustavo Padovan <gustavo.padovan at collabora.co.uk>
>>> ---
>>>  drivers/gpu/drm/exynos/Makefile             |   7 +-
>>>  drivers/gpu/drm/exynos/exynos7_drm_decon.c  |   2 +-
>>>  drivers/gpu/drm/exynos/exynos_dp_core.c     |  68 ++++++++++++------
>>>  drivers/gpu/drm/exynos/exynos_dp_core.h     |   2 +-
>>>  drivers/gpu/drm/exynos/exynos_drm_core.c    |   1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_crtc.c    |   1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_dpi.c     |  51 ++++++++------
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.c     |   1 -
>>>  drivers/gpu/drm/exynos/exynos_drm_drv.h     |  47 ++-----------
>>>  drivers/gpu/drm/exynos/exynos_drm_dsi.c     |  80 +++++++++++----------
>>>  drivers/gpu/drm/exynos/exynos_drm_encoder.c | 105 ----------------------------
>>>  drivers/gpu/drm/exynos/exynos_drm_encoder.h |  22 ------
>>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c    |   2 +-
>>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c    |  71 ++++++++++++++-----
>>>  drivers/gpu/drm/exynos/exynos_hdmi.c        |  85 +++++++++++++---------
>>>  15 files changed, 236 insertions(+), 309 deletions(-)
>>>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>>  delete mode 100644 drivers/gpu/drm/exynos/exynos_drm_encoder.h
>>
>> [-- SNIP --]
>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index d791ad4..a87d030 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -30,7 +30,6 @@
>>>  #include <video/videomode.h>
>>>  
>>>  #include "exynos_drm_crtc.h"
>>> -#include "exynos_drm_encoder.h"
>>>  #include "exynos_drm_drv.h"
>>>  
>>>  /* returns true iff both arguments logically differs */
>>> @@ -260,7 +259,7 @@ struct exynos_dsi_driver_data {
>>>  };
>>>  
>>>  struct exynos_dsi {
>>> -	struct exynos_drm_encoder encoder;
>>> +	struct drm_encoder encoder;
>>>  	struct mipi_dsi_host dsi_host;
>>>  	struct drm_connector connector;
>>>  	struct device_node *panel_node;
>>> @@ -296,7 +295,7 @@ struct exynos_dsi {
>>>  #define host_to_dsi(host) container_of(host, struct exynos_dsi, dsi_host)
>>>  #define connector_to_dsi(c) container_of(c, struct exynos_dsi, connector)
>>>  
>>> -static inline struct exynos_dsi *encoder_to_dsi(struct exynos_drm_encoder *e)
>>> +static inline struct exynos_dsi *encoder_to_dsi(struct drm_encoder *e)
>>>  {
>>>  	return container_of(e, struct exynos_dsi, encoder);
>>>  }
>>> @@ -1273,7 +1272,7 @@ static irqreturn_t exynos_dsi_irq(int irq, void *dev_id)
>>>  static irqreturn_t exynos_dsi_te_irq_handler(int irq, void *dev_id)
>>>  {
>>>  	struct exynos_dsi *dsi = (struct exynos_dsi *)dev_id;
>>> -	struct drm_encoder *encoder = &dsi->encoder.base;
>>> +	struct drm_encoder *encoder = &dsi->encoder;
>>>  
>>>  	if (dsi->state & DSIM_STATE_VIDOUT_AVAILABLE)
>>>  		exynos_drm_crtc_te_handler(encoder->crtc);
>>> @@ -1519,7 +1518,7 @@ static void exynos_dsi_poweroff(struct exynos_dsi *dsi)
>>>  		dev_err(dsi->dev, "cannot disable regulators %d\n", ret);
>>>  }
>>>  
>>> -static void exynos_dsi_enable(struct exynos_drm_encoder *encoder)
>>> +static void exynos_dsi_enable(struct drm_encoder *encoder)
>>>  {
>>>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>>>  	int ret;
>>> @@ -1555,7 +1554,7 @@ static void exynos_dsi_enable(struct exynos_drm_encoder *encoder)
>>>  	dsi->state |= DSIM_STATE_VIDOUT_AVAILABLE;
>>>  }
>>>  
>>> -static void exynos_dsi_disable(struct exynos_drm_encoder *encoder)
>>> +static void exynos_dsi_disable(struct drm_encoder *encoder)
>>>  {
>>>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>>>  
>>> @@ -1583,7 +1582,7 @@ exynos_dsi_detect(struct drm_connector *connector, bool force)
>>>  		if (dsi->panel)
>>>  			drm_panel_attach(dsi->panel, &dsi->connector);
>>>  	} else if (!dsi->panel_node) {
>>> -		struct exynos_drm_encoder *encoder;
>>> +		struct drm_encoder *encoder;
>>>  
>>>  		encoder = platform_get_drvdata(to_platform_device(dsi->dev));
>>>  		exynos_dsi_disable(encoder);
>>> @@ -1629,7 +1628,7 @@ exynos_dsi_best_encoder(struct drm_connector *connector)
>>>  {
>>>  	struct exynos_dsi *dsi = connector_to_dsi(connector);
>>>  
>>> -	return &dsi->encoder.base;
>>> +	return &dsi->encoder;
>>>  }
>>>  
>>>  static struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = {
>>> @@ -1637,11 +1636,9 @@ static struct drm_connector_helper_funcs exynos_dsi_connector_helper_funcs = {
>>>  	.best_encoder = exynos_dsi_best_encoder,
>>>  };
>>>  
>>> -static int exynos_dsi_create_connector(
>>> -				struct exynos_drm_encoder *exynos_encoder)
>>> +static int exynos_dsi_create_connector(struct drm_encoder *encoder)
>>>  {
>>> -	struct exynos_dsi *dsi = encoder_to_dsi(exynos_encoder);
>>> -	struct drm_encoder *encoder = &exynos_encoder->base;
>>> +	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>>>  	struct drm_connector *connector = &dsi->connector;
>>>  	int ret;
>>>  
>>> @@ -1662,28 +1659,34 @@ static int exynos_dsi_create_connector(
>>>  	return 0;
>>>  }
>>>  
>>> -static void exynos_dsi_mode_set(struct exynos_drm_encoder *encoder,
>>> -			 struct drm_display_mode *mode)
>>> +static void exynos_dsi_mode_set(struct drm_encoder *encoder,
>>> +				struct drm_display_mode *mode,
>>> +				struct drm_display_mode *adjusted_mode)
>>>  {
>>>  	struct exynos_dsi *dsi = encoder_to_dsi(encoder);
>>>  	struct videomode *vm = &dsi->vm;
>>> -
>>> -	vm->hactive = mode->hdisplay;
>>> -	vm->vactive = mode->vdisplay;
>>> -	vm->vfront_porch = mode->vsync_start - mode->vdisplay;
>>> -	vm->vback_porch = mode->vtotal - mode->vsync_end;
>>> -	vm->vsync_len = mode->vsync_end - mode->vsync_start;
>>> -	vm->hfront_porch = mode->hsync_start - mode->hdisplay;
>>> -	vm->hback_porch = mode->htotal - mode->hsync_end;
>>> -	vm->hsync_len = mode->hsync_end - mode->hsync_start;
>>> +	struct drm_display_mode *m = adjusted_mode;
>>> +
>>> +	vm->hactive = m->hdisplay;
>>> +	vm->vactive = m->vdisplay;
>>> +	vm->vfront_porch = m->vsync_start - m->vdisplay;
>>> +	vm->vback_porch = m->vtotal - m->vsync_end;
>>> +	vm->vsync_len = m->vsync_end - m->vsync_start;
>>> +	vm->hfront_porch = m->hsync_start - m->hdisplay;
>>> +	vm->hback_porch = m->htotal - m->hsync_end;
>>> +	vm->hsync_len = m->hsync_end - m->hsync_start;
>>
>> Above changes are not related to what this patch wants to do and just
>> cleanup patch.
> 
> It is related in my opinion. Here I change dsi mode_set() to be a
> drm_encoder ops. Before the change 'mode' was receiving ajusted_mode, but
> now 'mode' receives the original 'mode' and I need to add 
> 
> 	struct drm_display_mode *m = adjusted_mode;
> 
> to use adjusted_mode again.

You changed local variable name from 'mode' to 'm' even through using
adjusted_mode instead.

There would be no any changes if you declared like this, struct
drm_display_mode *mode = adjusted_mode. And seems simple enough and
meaningful more to use 'mode' variable name. Anyway, that is very
trivial issue but is there any reason to use 'm' instead of 'mode'?

Thanks,
Inki Dae

> 
> 	Gustavo
> 



More information about the dri-devel mailing list