2015-08-05 Inki Dae <inki.dae@xxxxxxxxxxx>:
Hi Inki,
> On 2015년 08월 04일 23:47, Gustavo Padovan wrote:
> > Hi Inki,
> >
> > 2015-08-04 Inki Dae <inki.dae@xxxxxxxxxxx>:
> >
> >> On 2015년 08월 04일 04:09, Gustavo Padovan wrote:
> >>> From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> >>>
> >>> 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@xxxxxxxxxxxxxxx>
> >>> ---
> >>> 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'?
Yes, because exynos_dsi_mode_set() already have 'mode' as argument:
static void exynos_dsi_mode_set(struct drm_encoder *encoder,
struct drm_display_mode *mode,
struct drm_display_mode *adjusted_mode)
Thus I need to define m = adjusted_mode
Gustavo
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
