| Message ID | 1424964285-27480-2-git-send-email-ajaykumar.rs@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/26/2015 04:24 PM, Ajay Kumar wrote: > * Modify DECON-INT driver to support DECON-EXT. > * Add a table of porch values needed to set timing registers of DECON-EXT. > * DECON-EXT supports only H/w Triggered COMMAND mode. > * DECON-EXT supports only one DMA window(window 1), so modify > all window management routines to support 2 windows of DECON-INT > and 1 window of DECON-EXT. > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > --- > .../devicetree/bindings/video/exynos7-decon.txt | 8 +- > drivers/gpu/drm/exynos/exynos7_drm_decon.c | 229 ++++++++++++++++++-- > include/video/exynos7_decon.h | 13 ++ > 3 files changed, 230 insertions(+), 20 deletions(-) > > diff --git a/Documentation/devicetree/bindings/video/exynos7-decon.txt b/Documentation/devicetree/bindings/video/exynos7-decon.txt > index f5f9c8d..87350c0 100644 > --- a/Documentation/devicetree/bindings/video/exynos7-decon.txt > +++ b/Documentation/devicetree/bindings/video/exynos7-decon.txt > @@ -2,10 +2,14 @@ Device-Tree bindings for Samsung Exynos7 SoC display controller (DECON) > > DECON (Display and Enhancement Controller) is the Display Controller for the > Exynos7 series of SoCs which transfers the image data from a video memory > -buffer to an external LCD interface. > +buffer to an external LCD/HDMI interface. > + > +Exynos7 supports DECON-INT for generating video signals for LCD. > +Exynos7 supports DECON-EXT for generating video signals for HDMI. > > Required properties: > -- compatible: value should be "samsung,exynos7-decon"; > +- compatible: value should be "samsung,exynos7-decon-int" for DECON-INT; > + value should be "samsung,exynos7-decon-ext" for DECON-EXT; > > - reg: physical base address and length of the DECON registers set. > > diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c > index 63f02e2..9e2d083 100644 > --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c > +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c > @@ -41,6 +41,28 @@ > > #define WINDOWS_NR 2 Maybe it would be better to rename it to MAX_WINDOWS_NR > > +#define DECON_EXT_CH_MAPPING 0x432105 I am not familiar with decon, could you explain what exactly this mapping means and why is it fixed in the driver to this value, not default one. By the way my specs says bits 0-3 are reserverd and here it seems you are using them. > + > +enum decon_type { > + DECON_INT, > + DECON_EXT, > +} decon_type; > + > +struct decon_driver_data { > + enum decon_type decon_type; > + unsigned int nr_windows; > +}; > + > +static struct decon_driver_data exynos7_decon_int_driver_data = { > + .decon_type = DECON_INT, I wonder if it wouldn't be better to use different fields to describe each capability/property instead of decon_type. For example .display_type = EXYNOS_DISPLAY_TYPE_LCD, .map_channels = 0, This way the code will be cleaner (less ifs). > + .nr_windows = 2, > +}; > + > +static struct decon_driver_data exynos7_decon_ext_driver_data = { > + .decon_type = DECON_EXT, > + .nr_windows = 1, > +}; > + > struct decon_win_data { > unsigned int ovl_x; > unsigned int ovl_y; > @@ -76,15 +98,28 @@ struct decon_context { > atomic_t wait_vsync_event; > > struct exynos_drm_panel_info panel; > + struct decon_driver_data *driver_data; > struct exynos_drm_display *display; > }; > > static const struct of_device_id decon_driver_dt_match[] = { > - {.compatible = "samsung,exynos7-decon"}, > + { .compatible = "samsung,exynos7-decon-int", > + .data = &exynos7_decon_int_driver_data }, > + { .compatible = "samsung,exynos7-decon-ext", > + .data = &exynos7_decon_ext_driver_data }, > {}, > }; > MODULE_DEVICE_TABLE(of, decon_driver_dt_match); > > +static inline struct decon_driver_data *drm_decon_get_driver_data( > + struct platform_device *pdev) > +{ > + const struct of_device_id *of_id = > + of_match_device(decon_driver_dt_match, &pdev->dev); > + > + return (struct decon_driver_data *)of_id->data; > +} > + > static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc) > { > struct decon_context *ctx = crtc->ctx; > @@ -106,13 +141,20 @@ static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc) > > static void decon_clear_channel(struct decon_context *ctx) > { > + struct decon_driver_data *drv_data = ctx->driver_data; > int win, ch_enabled = 0; > > DRM_DEBUG_KMS("%s\n", __FILE__); > > /* Check if any channel is enabled. */ > - for (win = 0; win < WINDOWS_NR; win++) { > - u32 val = readl(ctx->regs + WINCON(win)); > + for (win = 0; win < drv_data->nr_windows; win++) { > + u32 val; > + /* DECON EXT sw-hw window mapping */ > + if (drv_data->decon_type == DECON_EXT) { > + if (win == 0) > + win = 1; Changing value of for iterator inside the loop seems quite strange. Wouldn't be better to start loop from 1 in case of DECON_EXT. > + } > + val = readl(ctx->regs + WINCON(win)); > > if (val & WINCONx_ENWIN) { > val &= ~WINCONx_ENWIN; > @@ -187,10 +229,74 @@ static bool decon_mode_fixup(struct exynos_drm_crtc *crtc, > return true; > } > > +struct decon_ext_videomode { > + u32 vfront_porch; > + u32 vback_porch; > + u32 hfront_porch; > + u32 hback_porch; > + u32 vsync_len; > + u32 hsync_len; > + u32 hactive; > + u32 vactive; > + u32 refresh_rate; > +}; > + > +enum DECON_EXT_MODES { > + MODE_720_480_60, > + MODE_720_576_50, > + MODE_1280_720_60, > + MODE_1280_720_50, > + MODE_1920_1080_60, > + MODE_1920_1080_50, > + MODE_1920_1080_30, > + MODE_1920_1080_25, > + MODE_1920_1080_24, > + MODE_3840_2160_30, > + MODE_3840_2160_25, > + MODE_3840_2160_24, > + MODE_4096_2160_24, > +}; > + > +static struct decon_ext_videomode decon_ext_porchs[] = { > + /*vfp vbp hfp hbp vsa hsa xres yres fps */ > + {1, 43, 10, 92, 1, 36, 720, 480, 60}, > + {1, 47, 10, 92, 1, 42, 720, 576, 50}, > + {1, 28, 10, 192, 1, 168, 1280, 720, 60}, > + {1, 28, 10, 92, 1, 598, 1280, 720, 50}, > + {1, 43, 10, 92, 1, 178, 1920, 1080, 60}, > + {1, 43, 10, 92, 1, 618, 1920, 1080, 50}, > + {1, 43, 10, 92, 1, 178, 1920, 1080, 30}, > + {1, 43, 10, 92, 1, 618, 1920, 1080, 25}, > + {1, 43, 10, 92, 1, 728, 1920, 1080, 24}, > + {1, 88, 10, 458, 1, 92, 3840, 2160, 30}, > + {1, 88, 10, 1338, 1, 92, 3840, 2160, 25}, > + {1, 88, 10, 1558, 1, 92, 3840, 2160, 24}, > + {1, 88, 10, 1302, 1, 92, 4096, 2160, 24}, > +}; > + > +static void decon_ext_set_timing(struct drm_display_mode *mode, > + struct decon_ext_videomode *ext_mode) > +{ > + mode->crtc_hdisplay = ext_mode->hactive; > + mode->crtc_hsync_start = ext_mode->hactive + ext_mode->hfront_porch; > + mode->crtc_hsync_end = ext_mode->hactive + ext_mode->hfront_porch > + + ext_mode->hsync_len; > + mode->crtc_htotal = ext_mode->hactive + ext_mode->hfront_porch > + + ext_mode->hsync_len + ext_mode->hback_porch; > + mode->crtc_vdisplay = ext_mode->vactive; > + mode->crtc_vsync_start = ext_mode->vactive + ext_mode->vfront_porch; > + mode->crtc_vsync_end = ext_mode->vactive + ext_mode->vfront_porch > + + ext_mode->vsync_len; > + mode->crtc_vtotal = ext_mode->vactive + ext_mode->vfront_porch > + + ext_mode->vsync_len + ext_mode->vback_porch; > +} > + > static void decon_commit(struct exynos_drm_crtc *crtc) > { > struct decon_context *ctx = crtc->ctx; > + struct decon_driver_data *drv_data = ctx->driver_data; > struct drm_display_mode *mode = &crtc->base.mode; > + struct decon_ext_videomode *my_mode; > u32 val, clkdiv; > > if (ctx->suspended) > @@ -200,6 +306,38 @@ static void decon_commit(struct exynos_drm_crtc *crtc) > if (mode->htotal == 0 || mode->vtotal == 0) > return; > > + if (drv_data->decon_type == DECON_EXT) { > + if ((mode->hdisplay == 720) && (mode->vdisplay == 480) && > + (mode->vrefresh == 60)) > + my_mode = &decon_ext_porchs[MODE_720_480_60]; > + if ((mode->hdisplay == 720) && (mode->vdisplay == 576) && > + (mode->vrefresh == 50)) > + my_mode = &decon_ext_porchs[MODE_720_576_50]; > + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && > + (mode->vrefresh == 60)) > + my_mode = &decon_ext_porchs[MODE_1280_720_60]; > + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && > + (mode->vrefresh == 50)) > + my_mode = &decon_ext_porchs[MODE_1280_720_50]; > + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && > + (mode->vrefresh == 60)) > + my_mode = &decon_ext_porchs[MODE_1920_1080_60]; > + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && > + (mode->vrefresh == 50)) > + my_mode = &decon_ext_porchs[MODE_1920_1080_50]; > + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && > + (mode->vrefresh == 30)) > + my_mode = &decon_ext_porchs[MODE_1920_1080_30]; > + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && > + (mode->vrefresh == 25)) > + my_mode = &decon_ext_porchs[MODE_1920_1080_25]; > + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && > + (mode->vrefresh == 24)) > + my_mode = &decon_ext_porchs[MODE_1920_1080_24]; > + > + decon_ext_set_timing(mode, my_mode); > + } > + I guess these ugly ifs could be replaced by simple loop: for (i = 0; i < ARRAY_SIZE(decon_ext_porchs); ++i) { if (mode->hdisplay != decon_ext_porchs[i].hdisplay || ...) continue; decon_ext_set_timing(mode, &decon_ext_porchs[i]); break; } Anyway I have doubts if commit should modify crtc.base.mode I guess better place for it can be in *_fixup callback. > if (!ctx->i80_if) { > int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd; > /* setup vertical timing values. */ > @@ -301,6 +439,7 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, > { > struct decon_context *ctx = crtc->ctx; > struct decon_win_data *win_data; > + struct decon_driver_data *drv_data = ctx->driver_data; > int win, padding; > > if (!plane) { > @@ -312,12 +451,18 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, > if (win == DEFAULT_ZPOS) > win = ctx->default_win; > > - if (win < 0 || win >= WINDOWS_NR) > + if (win < 0 || win >= drv_data->nr_windows) > return; > > > win_data = &ctx->win_data[win]; > > + /* DECON EXT sw-hw window mapping */ > + if (drv_data->decon_type == DECON_EXT) { > + if (win == 0) > + win = 1; > + } This code is repeated few times, maybe it would be good to place it into some helper function. > + > padding = (plane->pitch / (plane->bpp >> 3)) - plane->fb_width; > win_data->offset_x = plane->fb_x; > win_data->offset_y = plane->fb_y; > @@ -340,9 +485,9 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, > plane->fb_width, plane->crtc_width); > } > > -static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win) > +static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, > + struct decon_win_data *win_data) > { > - struct decon_win_data *win_data = &ctx->win_data[win]; > unsigned long val; > > val = readl(ctx->regs + WINCON(win)); > @@ -454,6 +599,7 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) > struct decon_context *ctx = crtc->ctx; > struct drm_display_mode *mode = &crtc->base.mode; > struct decon_win_data *win_data; > + struct decon_driver_data *drv_data = ctx->driver_data; > int win = zpos; > unsigned long val, alpha; > unsigned int last_x; > @@ -465,11 +611,17 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) > if (win == DEFAULT_ZPOS) > win = ctx->default_win; > > - if (win < 0 || win >= WINDOWS_NR) > + if (win < 0 || win >= drv_data->nr_windows) > return; > > win_data = &ctx->win_data[win]; > > + /* DECON EXT sw-hw window mapping */ > + if (drv_data->decon_type == DECON_EXT) { > + if (win == 0) > + win = 1; > + } > + > /* If suspended, enable this on resume */ > if (ctx->suspended) { > win_data->resume = true; > @@ -546,7 +698,7 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) > > writel(alpha, ctx->regs + VIDOSD_D(win)); > > - decon_win_set_pixfmt(ctx, win); > + decon_win_set_pixfmt(ctx, win, win_data); > > /* hardware window 0 doesn't support color key. */ > if (win != 0) > @@ -572,17 +724,24 @@ static void decon_win_disable(struct exynos_drm_crtc *crtc, int zpos) > { > struct decon_context *ctx = crtc->ctx; > struct decon_win_data *win_data; > + struct decon_driver_data *drv_data = ctx->driver_data; > int win = zpos; > u32 val; > > if (win == DEFAULT_ZPOS) > win = ctx->default_win; > > - if (win < 0 || win >= WINDOWS_NR) > + if (win < 0 || win >= drv_data->nr_windows) > return; > > win_data = &ctx->win_data[win]; > > + /* DECON EXT sw-hw window mapping */ > + if (drv_data->decon_type == DECON_EXT) { > + if (win == 0) > + win = 1; > + } > + > if (ctx->suspended) { > /* do not resume this window*/ > win_data->resume = false; > @@ -609,10 +768,11 @@ static void decon_win_disable(struct exynos_drm_crtc *crtc, int zpos) > > static void decon_window_suspend(struct decon_context *ctx) > { > + struct decon_driver_data *drv_data = ctx->driver_data; > struct decon_win_data *win_data; > int i; > > - for (i = 0; i < WINDOWS_NR; i++) { > + for (i = 0; i < drv_data->nr_windows; i++) { > win_data = &ctx->win_data[i]; > win_data->resume = win_data->enabled; > if (win_data->enabled) > @@ -622,10 +782,11 @@ static void decon_window_suspend(struct decon_context *ctx) > > static void decon_window_resume(struct decon_context *ctx) > { > + struct decon_driver_data *drv_data = ctx->driver_data; > struct decon_win_data *win_data; > int i; > > - for (i = 0; i < WINDOWS_NR; i++) { > + for (i = 0; i < drv_data->nr_windows; i++) { > win_data = &ctx->win_data[i]; > win_data->enabled = win_data->resume; > win_data->resume = false; > @@ -634,10 +795,11 @@ static void decon_window_resume(struct decon_context *ctx) > > static void decon_apply(struct decon_context *ctx) > { > + struct decon_driver_data *drv_data = ctx->driver_data; > struct decon_win_data *win_data; > int i; > > - for (i = 0; i < WINDOWS_NR; i++) { > + for (i = 0; i < drv_data->nr_windows; i++) { > win_data = &ctx->win_data[i]; > if (win_data->enabled) > decon_win_commit(ctx->crtc, i); > @@ -650,6 +812,7 @@ static void decon_apply(struct decon_context *ctx) > > static void decon_init(struct decon_context *ctx) > { > + struct decon_driver_data *drv_data = ctx->driver_data; > u32 val; > > writel(VIDCON0_SWRESET, ctx->regs + VIDCON0); > @@ -657,6 +820,14 @@ static void decon_init(struct decon_context *ctx) > val = VIDOUTCON0_DISP_IF_0_ON; > if (!ctx->i80_if) > val |= VIDOUTCON0_RGBIF; > + > + if (drv_data->decon_type == DECON_EXT) { > + writel(TRIGCON_HWTRIG_AUTO_MASK | > + TRIGCON_HWTRIGMASK_DISPIF0 | > + TRIGCON_HWTRIGEN_I80_RGB, ctx->regs + TRIGCON); > + val |= VIDOUTCON0_TV_MODE | VIDOUTCON0_I80IF; > + } > + > writel(val, ctx->regs + VIDOUTCON0); > > writel(VCLKCON0_CLKVALUP | VCLKCON0_VCLKFREE, ctx->regs + VCLKCON0); > @@ -667,6 +838,7 @@ static void decon_init(struct decon_context *ctx) > > static int decon_poweron(struct decon_context *ctx) > { > + struct decon_driver_data *drv_data = ctx->driver_data; > int ret; > > if (!ctx->suspended) > @@ -702,6 +874,9 @@ static int decon_poweron(struct decon_context *ctx) > > decon_init(ctx); > > + if (drv_data->decon_type == DECON_EXT) > + writel(DECON_EXT_CH_MAPPING, ctx->regs + WINCHMAP0); > + > /* if vblank was enabled status, enable it again. */ > if (test_and_clear_bit(0, &ctx->irq_flags)) { > ret = decon_enable_vblank(ctx->crtc); > @@ -817,6 +992,7 @@ out: > static int decon_bind(struct device *dev, struct device *master, void *data) > { > struct decon_context *ctx = dev_get_drvdata(dev); > + struct decon_driver_data *drv_data = ctx->driver_data; > struct drm_device *drm_dev = data; > int ret; > > @@ -826,16 +1002,23 @@ static int decon_bind(struct device *dev, struct device *master, void *data) > return ret; > } > > - ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, > + if (drv_data->decon_type == DECON_INT) > + ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, > EXYNOS_DISPLAY_TYPE_LCD, > &decon_crtc_ops, ctx); > + else > + ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, > + EXYNOS_DISPLAY_TYPE_HDMI, > + &decon_crtc_ops, ctx); > + > if (IS_ERR(ctx->crtc)) { > decon_ctx_remove(ctx); > return PTR_ERR(ctx->crtc); > } > > - if (ctx->display) > - exynos_drm_create_enc_conn(drm_dev, ctx->display); > + if (drv_data->decon_type == DECON_INT) This check should not be necessary, ctx->display shall be set only in case of presence of parallel output. > + if (ctx->display) > + exynos_drm_create_enc_conn(drm_dev, ctx->display); > > return 0; > > @@ -848,8 +1031,9 @@ static void decon_unbind(struct device *dev, struct device *master, > > decon_dpms(ctx->crtc, DRM_MODE_DPMS_OFF); > > - if (ctx->display) > - exynos_dpi_remove(ctx->display); > + if (ctx->driver_data->decon_type == DECON_INT) ditto Regards Andrzej > + if (ctx->display) > + exynos_dpi_remove(ctx->display); > > decon_ctx_remove(ctx); > } > @@ -862,11 +1046,14 @@ static const struct component_ops decon_component_ops = { > static int decon_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > + struct decon_driver_data *drv_data; > struct decon_context *ctx; > struct device_node *i80_if_timings; > struct resource *res; > int ret; > > + drv_data = drm_decon_get_driver_data(pdev); > + > if (!dev->of_node) > return -ENODEV; > > @@ -874,11 +1061,17 @@ static int decon_probe(struct platform_device *pdev) > if (!ctx) > return -ENOMEM; > > - ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, > + if (drv_data->decon_type == DECON_INT) > + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, > EXYNOS_DISPLAY_TYPE_LCD); > + else > + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, > + EXYNOS_DISPLAY_TYPE_HDMI); > + > if (ret) > return ret; > > + ctx->driver_data = drv_data; > ctx->dev = dev; > ctx->suspended = true; > > diff --git a/include/video/exynos7_decon.h b/include/video/exynos7_decon.h > index a62b11b..d02320b 100644 > --- a/include/video/exynos7_decon.h > +++ b/include/video/exynos7_decon.h > @@ -20,6 +20,7 @@ > /* VIDOUTCON0 */ > #define VIDOUTCON0 0x4 > > +#define VIDOUTCON0_TV_MODE (0x1 << 26) > #define VIDOUTCON0_DUAL_MASK (0x3 << 24) > #define VIDOUTCON0_DUAL_ON (0x3 << 24) > #define VIDOUTCON0_DISP_IF_1_ON (0x2 << 24) > @@ -52,6 +53,9 @@ > > #define SHADOWCON_WINx_PROTECT(_win) (1 << (10 + (_win))) > > +/* WINCHMAP0 */ > +#define WINCHMAP0 0x40 > + > /* WINCONx */ > #define WINCON(_win) (0x50 + ((_win) * 4)) > > @@ -328,6 +332,15 @@ > /* LINECNT OP THRSHOLD*/ > #define LINECNT_OP_THRESHOLD 0x630 > > +/* TRIGCON */ > +#define TRIGCON 0x06B0 > +#define TRIGCON_HWTRIG_AUTO_MASK (1 << 6) > +#define TRIGCON_HWTRIGMASK_DISPIF1 (1 << 5) > +#define TRIGCON_HWTRIGMASK_DISPIF0 (1 << 4) > +#define TRIGCON_HWTRIGEN_I80_RGB (1 << 3) > +#define TRIGCON_SWTRIGCMD_I80_RGB (1 << 1) > +#define TRIGCON_SWTRIGEN_I80_RGB (1 << 0) > + > /* CRCCTRL */ > #define CRCCTRL 0x6C8 > #define CRCCTRL_CRCCLKEN (0x1 << 2) >
On Mon, Mar 2, 2015 at 1:38 PM, Andrzej Hajda <a.hajda@samsung.com> wrote: > On 02/26/2015 04:24 PM, Ajay Kumar wrote: >> * Modify DECON-INT driver to support DECON-EXT. >> * Add a table of porch values needed to set timing registers of DECON-EXT. >> * DECON-EXT supports only H/w Triggered COMMAND mode. >> * DECON-EXT supports only one DMA window(window 1), so modify >> all window management routines to support 2 windows of DECON-INT >> and 1 window of DECON-EXT. >> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >> --- >> .../devicetree/bindings/video/exynos7-decon.txt | 8 +- >> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 229 ++++++++++++++++++-- >> include/video/exynos7_decon.h | 13 ++ >> 3 files changed, 230 insertions(+), 20 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/video/exynos7-decon.txt b/Documentation/devicetree/bindings/video/exynos7-decon.txt >> index f5f9c8d..87350c0 100644 >> --- a/Documentation/devicetree/bindings/video/exynos7-decon.txt >> +++ b/Documentation/devicetree/bindings/video/exynos7-decon.txt >> @@ -2,10 +2,14 @@ Device-Tree bindings for Samsung Exynos7 SoC display controller (DECON) >> >> DECON (Display and Enhancement Controller) is the Display Controller for the >> Exynos7 series of SoCs which transfers the image data from a video memory >> -buffer to an external LCD interface. >> +buffer to an external LCD/HDMI interface. >> + >> +Exynos7 supports DECON-INT for generating video signals for LCD. >> +Exynos7 supports DECON-EXT for generating video signals for HDMI. >> >> Required properties: >> -- compatible: value should be "samsung,exynos7-decon"; >> +- compatible: value should be "samsung,exynos7-decon-int" for DECON-INT; >> + value should be "samsung,exynos7-decon-ext" for DECON-EXT; >> >> - reg: physical base address and length of the DECON registers set. >> >> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c >> index 63f02e2..9e2d083 100644 >> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c >> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c >> @@ -41,6 +41,28 @@ >> >> #define WINDOWS_NR 2 > > Maybe it would be better to rename it to MAX_WINDOWS_NR Ok. >> >> +#define DECON_EXT_CH_MAPPING 0x432105 > > I am not familiar with decon, could you explain what exactly > this mapping means and why is it fixed in the driver to this value, > not default one. By the way my specs says bits 0-3 are reserverd > and here it seems you are using them. I reused the value from the code which hardware team shared with me. It tries to map a input hardware channel(IDMA or VPP channel) onto a hardware window in DECON. Channel_N is mapped onto window_N in case of DECON-INT. In case of DECON-EXT, Channel 0 should be mapped to window 1 of DECON-EXT. So, basically for the changes I have made, I should ideally set only bits [4:6] to 0x1, and leave other bits untouched, though modifying other bits wouldn't affect in anyway. > >> + >> +enum decon_type { >> + DECON_INT, >> + DECON_EXT, >> +} decon_type; >> + >> +struct decon_driver_data { >> + enum decon_type decon_type; >> + unsigned int nr_windows; >> +}; >> + >> +static struct decon_driver_data exynos7_decon_int_driver_data = { >> + .decon_type = DECON_INT, > > I wonder if it wouldn't be better to use different fields to describe > each capability/property instead of decon_type. For example > .display_type = EXYNOS_DISPLAY_TYPE_LCD, > .map_channels = 0, Ok, let me list down the differences between INT and EXT. 1) Only h/w triggered command mode for DECON-EXT. 2) Need to feed modified porch values(decon_ext_timings) 3) Input channel to H/w window mapping(WINCHMAP) 4) default_window - 0 for DECON-INT and 1 for DECON-EXT Out of the above differences, the first 3 can somehow be converted to capability/property and embedded into driver_data. But the 4th difference is bothering me. I tried using something like start_window, end_window and tried to make The code common for DECON-INT and DECON-EXT, and it just doesn't work. ex: start_window = 0, end_window = 1 for DECON-INT start_window = 1, end_window = 1 for DECON-EXT When win_commit gets called for window 0 from crtc_commit/plane_commit: Configure start_window(0 for DECON-INT and 1 for DECON-EXT) When win_commit is called from decon_apply, it is called for window 1 also. That time win_commit can skip updating actual window 1. How do I take care of this ambiguity? This case happens with win_disable routine also! > This way the code will be cleaner (less ifs). > > >> + .nr_windows = 2, >> +}; >> + >> +static struct decon_driver_data exynos7_decon_ext_driver_data = { >> + .decon_type = DECON_EXT, >> + .nr_windows = 1, >> +}; >> + >> struct decon_win_data { >> unsigned int ovl_x; >> unsigned int ovl_y; >> @@ -76,15 +98,28 @@ struct decon_context { >> atomic_t wait_vsync_event; >> >> struct exynos_drm_panel_info panel; >> + struct decon_driver_data *driver_data; >> struct exynos_drm_display *display; >> }; >> >> static const struct of_device_id decon_driver_dt_match[] = { >> - {.compatible = "samsung,exynos7-decon"}, >> + { .compatible = "samsung,exynos7-decon-int", >> + .data = &exynos7_decon_int_driver_data }, >> + { .compatible = "samsung,exynos7-decon-ext", >> + .data = &exynos7_decon_ext_driver_data }, >> {}, >> }; >> MODULE_DEVICE_TABLE(of, decon_driver_dt_match); >> >> +static inline struct decon_driver_data *drm_decon_get_driver_data( >> + struct platform_device *pdev) >> +{ >> + const struct of_device_id *of_id = >> + of_match_device(decon_driver_dt_match, &pdev->dev); >> + >> + return (struct decon_driver_data *)of_id->data; >> +} >> + >> static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc) >> { >> struct decon_context *ctx = crtc->ctx; >> @@ -106,13 +141,20 @@ static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc) >> >> static void decon_clear_channel(struct decon_context *ctx) >> { >> + struct decon_driver_data *drv_data = ctx->driver_data; >> int win, ch_enabled = 0; >> >> DRM_DEBUG_KMS("%s\n", __FILE__); >> >> /* Check if any channel is enabled. */ >> - for (win = 0; win < WINDOWS_NR; win++) { >> - u32 val = readl(ctx->regs + WINCON(win)); >> + for (win = 0; win < drv_data->nr_windows; win++) { >> + u32 val; >> + /* DECON EXT sw-hw window mapping */ >> + if (drv_data->decon_type == DECON_EXT) { >> + if (win == 0) >> + win = 1; > > Changing value of for iterator inside the loop seems quite strange. > Wouldn't be better to start loop from 1 in case of DECON_EXT. I have explained the resulting ambiguity above. > >> + } >> + val = readl(ctx->regs + WINCON(win)); >> >> if (val & WINCONx_ENWIN) { >> val &= ~WINCONx_ENWIN; >> @@ -187,10 +229,74 @@ static bool decon_mode_fixup(struct exynos_drm_crtc *crtc, >> return true; >> } >> >> +struct decon_ext_videomode { >> + u32 vfront_porch; >> + u32 vback_porch; >> + u32 hfront_porch; >> + u32 hback_porch; >> + u32 vsync_len; >> + u32 hsync_len; >> + u32 hactive; >> + u32 vactive; >> + u32 refresh_rate; >> +}; >> + >> +enum DECON_EXT_MODES { >> + MODE_720_480_60, >> + MODE_720_576_50, >> + MODE_1280_720_60, >> + MODE_1280_720_50, >> + MODE_1920_1080_60, >> + MODE_1920_1080_50, >> + MODE_1920_1080_30, >> + MODE_1920_1080_25, >> + MODE_1920_1080_24, >> + MODE_3840_2160_30, >> + MODE_3840_2160_25, >> + MODE_3840_2160_24, >> + MODE_4096_2160_24, >> +}; >> + >> +static struct decon_ext_videomode decon_ext_porchs[] = { >> + /*vfp vbp hfp hbp vsa hsa xres yres fps */ >> + {1, 43, 10, 92, 1, 36, 720, 480, 60}, >> + {1, 47, 10, 92, 1, 42, 720, 576, 50}, >> + {1, 28, 10, 192, 1, 168, 1280, 720, 60}, >> + {1, 28, 10, 92, 1, 598, 1280, 720, 50}, >> + {1, 43, 10, 92, 1, 178, 1920, 1080, 60}, >> + {1, 43, 10, 92, 1, 618, 1920, 1080, 50}, >> + {1, 43, 10, 92, 1, 178, 1920, 1080, 30}, >> + {1, 43, 10, 92, 1, 618, 1920, 1080, 25}, >> + {1, 43, 10, 92, 1, 728, 1920, 1080, 24}, >> + {1, 88, 10, 458, 1, 92, 3840, 2160, 30}, >> + {1, 88, 10, 1338, 1, 92, 3840, 2160, 25}, >> + {1, 88, 10, 1558, 1, 92, 3840, 2160, 24}, >> + {1, 88, 10, 1302, 1, 92, 4096, 2160, 24}, >> +}; >> + >> +static void decon_ext_set_timing(struct drm_display_mode *mode, >> + struct decon_ext_videomode *ext_mode) >> +{ >> + mode->crtc_hdisplay = ext_mode->hactive; >> + mode->crtc_hsync_start = ext_mode->hactive + ext_mode->hfront_porch; >> + mode->crtc_hsync_end = ext_mode->hactive + ext_mode->hfront_porch >> + + ext_mode->hsync_len; >> + mode->crtc_htotal = ext_mode->hactive + ext_mode->hfront_porch >> + + ext_mode->hsync_len + ext_mode->hback_porch; >> + mode->crtc_vdisplay = ext_mode->vactive; >> + mode->crtc_vsync_start = ext_mode->vactive + ext_mode->vfront_porch; >> + mode->crtc_vsync_end = ext_mode->vactive + ext_mode->vfront_porch >> + + ext_mode->vsync_len; >> + mode->crtc_vtotal = ext_mode->vactive + ext_mode->vfront_porch >> + + ext_mode->vsync_len + ext_mode->vback_porch; >> +} >> + >> static void decon_commit(struct exynos_drm_crtc *crtc) >> { >> struct decon_context *ctx = crtc->ctx; >> + struct decon_driver_data *drv_data = ctx->driver_data; >> struct drm_display_mode *mode = &crtc->base.mode; >> + struct decon_ext_videomode *my_mode; >> u32 val, clkdiv; >> >> if (ctx->suspended) >> @@ -200,6 +306,38 @@ static void decon_commit(struct exynos_drm_crtc *crtc) >> if (mode->htotal == 0 || mode->vtotal == 0) >> return; >> >> + if (drv_data->decon_type == DECON_EXT) { >> + if ((mode->hdisplay == 720) && (mode->vdisplay == 480) && >> + (mode->vrefresh == 60)) >> + my_mode = &decon_ext_porchs[MODE_720_480_60]; >> + if ((mode->hdisplay == 720) && (mode->vdisplay == 576) && >> + (mode->vrefresh == 50)) >> + my_mode = &decon_ext_porchs[MODE_720_576_50]; >> + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && >> + (mode->vrefresh == 60)) >> + my_mode = &decon_ext_porchs[MODE_1280_720_60]; >> + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && >> + (mode->vrefresh == 50)) >> + my_mode = &decon_ext_porchs[MODE_1280_720_50]; >> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >> + (mode->vrefresh == 60)) >> + my_mode = &decon_ext_porchs[MODE_1920_1080_60]; >> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >> + (mode->vrefresh == 50)) >> + my_mode = &decon_ext_porchs[MODE_1920_1080_50]; >> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >> + (mode->vrefresh == 30)) >> + my_mode = &decon_ext_porchs[MODE_1920_1080_30]; >> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >> + (mode->vrefresh == 25)) >> + my_mode = &decon_ext_porchs[MODE_1920_1080_25]; >> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >> + (mode->vrefresh == 24)) >> + my_mode = &decon_ext_porchs[MODE_1920_1080_24]; >> + >> + decon_ext_set_timing(mode, my_mode); >> + } >> + > > I guess these ugly ifs could be replaced by simple loop: > for (i = 0; i < ARRAY_SIZE(decon_ext_porchs); ++i) { > if (mode->hdisplay != decon_ext_porchs[i].hdisplay || ...) > continue; > decon_ext_set_timing(mode, &decon_ext_porchs[i]); > break; > } Ok, Though the number of checks is the same, your code at least looks better. :) > Anyway I have doubts if commit should modify crtc.base.mode I guess > better place for it can be in *_fixup callback. Inki? What is your suggestion? >> if (!ctx->i80_if) { >> int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd; >> /* setup vertical timing values. */ >> @@ -301,6 +439,7 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, >> { >> struct decon_context *ctx = crtc->ctx; >> struct decon_win_data *win_data; >> + struct decon_driver_data *drv_data = ctx->driver_data; >> int win, padding; >> >> if (!plane) { >> @@ -312,12 +451,18 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, >> if (win == DEFAULT_ZPOS) >> win = ctx->default_win; >> >> - if (win < 0 || win >= WINDOWS_NR) >> + if (win < 0 || win >= drv_data->nr_windows) >> return; >> >> >> win_data = &ctx->win_data[win]; >> >> + /* DECON EXT sw-hw window mapping */ >> + if (drv_data->decon_type == DECON_EXT) { >> + if (win == 0) >> + win = 1; >> + } > > This code is repeated few times, maybe it would be good to place it into > some helper function. Ok. >> + >> padding = (plane->pitch / (plane->bpp >> 3)) - plane->fb_width; >> win_data->offset_x = plane->fb_x; >> win_data->offset_y = plane->fb_y; >> @@ -340,9 +485,9 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, >> plane->fb_width, plane->crtc_width); >> } >> >> -static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win) >> +static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, >> + struct decon_win_data *win_data) >> { >> - struct decon_win_data *win_data = &ctx->win_data[win]; >> unsigned long val; >> >> val = readl(ctx->regs + WINCON(win)); >> @@ -454,6 +599,7 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) >> struct decon_context *ctx = crtc->ctx; >> struct drm_display_mode *mode = &crtc->base.mode; >> struct decon_win_data *win_data; >> + struct decon_driver_data *drv_data = ctx->driver_data; >> int win = zpos; >> unsigned long val, alpha; >> unsigned int last_x; >> @@ -465,11 +611,17 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) >> if (win == DEFAULT_ZPOS) >> win = ctx->default_win; >> >> - if (win < 0 || win >= WINDOWS_NR) >> + if (win < 0 || win >= drv_data->nr_windows) >> return; >> >> win_data = &ctx->win_data[win]; >> >> + /* DECON EXT sw-hw window mapping */ >> + if (drv_data->decon_type == DECON_EXT) { >> + if (win == 0) >> + win = 1; >> + } >> + >> /* If suspended, enable this on resume */ >> if (ctx->suspended) { >> win_data->resume = true; >> @@ -546,7 +698,7 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) >> >> writel(alpha, ctx->regs + VIDOSD_D(win)); >> >> - decon_win_set_pixfmt(ctx, win); >> + decon_win_set_pixfmt(ctx, win, win_data); >> >> /* hardware window 0 doesn't support color key. */ >> if (win != 0) >> @@ -572,17 +724,24 @@ static void decon_win_disable(struct exynos_drm_crtc *crtc, int zpos) >> { >> struct decon_context *ctx = crtc->ctx; >> struct decon_win_data *win_data; >> + struct decon_driver_data *drv_data = ctx->driver_data; >> int win = zpos; >> u32 val; >> >> if (win == DEFAULT_ZPOS) >> win = ctx->default_win; >> >> - if (win < 0 || win >= WINDOWS_NR) >> + if (win < 0 || win >= drv_data->nr_windows) >> return; >> >> win_data = &ctx->win_data[win]; >> >> + /* DECON EXT sw-hw window mapping */ >> + if (drv_data->decon_type == DECON_EXT) { >> + if (win == 0) >> + win = 1; >> + } >> + >> if (ctx->suspended) { >> /* do not resume this window*/ >> win_data->resume = false; >> @@ -609,10 +768,11 @@ static void decon_win_disable(struct exynos_drm_crtc *crtc, int zpos) >> >> static void decon_window_suspend(struct decon_context *ctx) >> { >> + struct decon_driver_data *drv_data = ctx->driver_data; >> struct decon_win_data *win_data; >> int i; >> >> - for (i = 0; i < WINDOWS_NR; i++) { >> + for (i = 0; i < drv_data->nr_windows; i++) { >> win_data = &ctx->win_data[i]; >> win_data->resume = win_data->enabled; >> if (win_data->enabled) >> @@ -622,10 +782,11 @@ static void decon_window_suspend(struct decon_context *ctx) >> >> static void decon_window_resume(struct decon_context *ctx) >> { >> + struct decon_driver_data *drv_data = ctx->driver_data; >> struct decon_win_data *win_data; >> int i; >> >> - for (i = 0; i < WINDOWS_NR; i++) { >> + for (i = 0; i < drv_data->nr_windows; i++) { >> win_data = &ctx->win_data[i]; >> win_data->enabled = win_data->resume; >> win_data->resume = false; >> @@ -634,10 +795,11 @@ static void decon_window_resume(struct decon_context *ctx) >> >> static void decon_apply(struct decon_context *ctx) >> { >> + struct decon_driver_data *drv_data = ctx->driver_data; >> struct decon_win_data *win_data; >> int i; >> >> - for (i = 0; i < WINDOWS_NR; i++) { >> + for (i = 0; i < drv_data->nr_windows; i++) { >> win_data = &ctx->win_data[i]; >> if (win_data->enabled) >> decon_win_commit(ctx->crtc, i); >> @@ -650,6 +812,7 @@ static void decon_apply(struct decon_context *ctx) >> >> static void decon_init(struct decon_context *ctx) >> { >> + struct decon_driver_data *drv_data = ctx->driver_data; >> u32 val; >> >> writel(VIDCON0_SWRESET, ctx->regs + VIDCON0); >> @@ -657,6 +820,14 @@ static void decon_init(struct decon_context *ctx) >> val = VIDOUTCON0_DISP_IF_0_ON; >> if (!ctx->i80_if) >> val |= VIDOUTCON0_RGBIF; >> + >> + if (drv_data->decon_type == DECON_EXT) { >> + writel(TRIGCON_HWTRIG_AUTO_MASK | >> + TRIGCON_HWTRIGMASK_DISPIF0 | >> + TRIGCON_HWTRIGEN_I80_RGB, ctx->regs + TRIGCON); >> + val |= VIDOUTCON0_TV_MODE | VIDOUTCON0_I80IF; >> + } >> + >> writel(val, ctx->regs + VIDOUTCON0); >> >> writel(VCLKCON0_CLKVALUP | VCLKCON0_VCLKFREE, ctx->regs + VCLKCON0); >> @@ -667,6 +838,7 @@ static void decon_init(struct decon_context *ctx) >> >> static int decon_poweron(struct decon_context *ctx) >> { >> + struct decon_driver_data *drv_data = ctx->driver_data; >> int ret; >> >> if (!ctx->suspended) >> @@ -702,6 +874,9 @@ static int decon_poweron(struct decon_context *ctx) >> >> decon_init(ctx); >> >> + if (drv_data->decon_type == DECON_EXT) >> + writel(DECON_EXT_CH_MAPPING, ctx->regs + WINCHMAP0); >> + >> /* if vblank was enabled status, enable it again. */ >> if (test_and_clear_bit(0, &ctx->irq_flags)) { >> ret = decon_enable_vblank(ctx->crtc); >> @@ -817,6 +992,7 @@ out: >> static int decon_bind(struct device *dev, struct device *master, void *data) >> { >> struct decon_context *ctx = dev_get_drvdata(dev); >> + struct decon_driver_data *drv_data = ctx->driver_data; >> struct drm_device *drm_dev = data; >> int ret; >> >> @@ -826,16 +1002,23 @@ static int decon_bind(struct device *dev, struct device *master, void *data) >> return ret; >> } >> >> - ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, >> + if (drv_data->decon_type == DECON_INT) >> + ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, >> EXYNOS_DISPLAY_TYPE_LCD, >> &decon_crtc_ops, ctx); >> + else >> + ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, >> + EXYNOS_DISPLAY_TYPE_HDMI, >> + &decon_crtc_ops, ctx); >> + >> if (IS_ERR(ctx->crtc)) { >> decon_ctx_remove(ctx); >> return PTR_ERR(ctx->crtc); >> } >> >> - if (ctx->display) >> - exynos_drm_create_enc_conn(drm_dev, ctx->display); >> + if (drv_data->decon_type == DECON_INT) > > This check should not be necessary, ctx->display shall be set only in > case of presence of parallel output. Ok, this check is not necessarily needed! Ajay >> + if (ctx->display) >> + exynos_drm_create_enc_conn(drm_dev, ctx->display); >> >> return 0; >> >> @@ -848,8 +1031,9 @@ static void decon_unbind(struct device *dev, struct device *master, >> >> decon_dpms(ctx->crtc, DRM_MODE_DPMS_OFF); >> >> - if (ctx->display) >> - exynos_dpi_remove(ctx->display); >> + if (ctx->driver_data->decon_type == DECON_INT) > > ditto > > > Regards > Andrzej > >> + if (ctx->display) >> + exynos_dpi_remove(ctx->display); >> >> decon_ctx_remove(ctx); >> } >> @@ -862,11 +1046,14 @@ static const struct component_ops decon_component_ops = { >> static int decon_probe(struct platform_device *pdev) >> { >> struct device *dev = &pdev->dev; >> + struct decon_driver_data *drv_data; >> struct decon_context *ctx; >> struct device_node *i80_if_timings; >> struct resource *res; >> int ret; >> >> + drv_data = drm_decon_get_driver_data(pdev); >> + >> if (!dev->of_node) >> return -ENODEV; >> >> @@ -874,11 +1061,17 @@ static int decon_probe(struct platform_device *pdev) >> if (!ctx) >> return -ENOMEM; >> >> - ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, >> + if (drv_data->decon_type == DECON_INT) >> + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, >> EXYNOS_DISPLAY_TYPE_LCD); >> + else >> + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, >> + EXYNOS_DISPLAY_TYPE_HDMI); >> + >> if (ret) >> return ret; >> >> + ctx->driver_data = drv_data; >> ctx->dev = dev; >> ctx->suspended = true; >> >> diff --git a/include/video/exynos7_decon.h b/include/video/exynos7_decon.h >> index a62b11b..d02320b 100644 >> --- a/include/video/exynos7_decon.h >> +++ b/include/video/exynos7_decon.h >> @@ -20,6 +20,7 @@ >> /* VIDOUTCON0 */ >> #define VIDOUTCON0 0x4 >> >> +#define VIDOUTCON0_TV_MODE (0x1 << 26) >> #define VIDOUTCON0_DUAL_MASK (0x3 << 24) >> #define VIDOUTCON0_DUAL_ON (0x3 << 24) >> #define VIDOUTCON0_DISP_IF_1_ON (0x2 << 24) >> @@ -52,6 +53,9 @@ >> >> #define SHADOWCON_WINx_PROTECT(_win) (1 << (10 + (_win))) >> >> +/* WINCHMAP0 */ >> +#define WINCHMAP0 0x40 >> + >> /* WINCONx */ >> #define WINCON(_win) (0x50 + ((_win) * 4)) >> >> @@ -328,6 +332,15 @@ >> /* LINECNT OP THRSHOLD*/ >> #define LINECNT_OP_THRESHOLD 0x630 >> >> +/* TRIGCON */ >> +#define TRIGCON 0x06B0 >> +#define TRIGCON_HWTRIG_AUTO_MASK (1 << 6) >> +#define TRIGCON_HWTRIGMASK_DISPIF1 (1 << 5) >> +#define TRIGCON_HWTRIGMASK_DISPIF0 (1 << 4) >> +#define TRIGCON_HWTRIGEN_I80_RGB (1 << 3) >> +#define TRIGCON_SWTRIGCMD_I80_RGB (1 << 1) >> +#define TRIGCON_SWTRIGEN_I80_RGB (1 << 0) >> + >> /* CRCCTRL */ >> #define CRCCTRL 0x6C8 >> #define CRCCTRL_CRCCLKEN (0x1 << 2) >> >
Hi, 2015-03-02 19:57 GMT+09:00 Ajay kumar <ajaynumb@gmail.com>: > On Mon, Mar 2, 2015 at 1:38 PM, Andrzej Hajda <a.hajda@samsung.com> wrote: >> On 02/26/2015 04:24 PM, Ajay Kumar wrote: >>> * Modify DECON-INT driver to support DECON-EXT. >>> * Add a table of porch values needed to set timing registers of DECON-EXT. >>> * DECON-EXT supports only H/w Triggered COMMAND mode. >>> * DECON-EXT supports only one DMA window(window 1), so modify >>> all window management routines to support 2 windows of DECON-INT >>> and 1 window of DECON-EXT. >>> >>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >>> --- >>> .../devicetree/bindings/video/exynos7-decon.txt | 8 +- >>> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 229 ++++++++++++++++++-- >>> include/video/exynos7_decon.h | 13 ++ >>> 3 files changed, 230 insertions(+), 20 deletions(-) >>> -- snip -- >>> static void decon_commit(struct exynos_drm_crtc *crtc) >>> { >>> struct decon_context *ctx = crtc->ctx; >>> + struct decon_driver_data *drv_data = ctx->driver_data; >>> struct drm_display_mode *mode = &crtc->base.mode; >>> + struct decon_ext_videomode *my_mode; >>> u32 val, clkdiv; >>> >>> if (ctx->suspended) >>> @@ -200,6 +306,38 @@ static void decon_commit(struct exynos_drm_crtc *crtc) >>> if (mode->htotal == 0 || mode->vtotal == 0) >>> return; >>> >>> + if (drv_data->decon_type == DECON_EXT) { >>> + if ((mode->hdisplay == 720) && (mode->vdisplay == 480) && >>> + (mode->vrefresh == 60)) >>> + my_mode = &decon_ext_porchs[MODE_720_480_60]; >>> + if ((mode->hdisplay == 720) && (mode->vdisplay == 576) && >>> + (mode->vrefresh == 50)) >>> + my_mode = &decon_ext_porchs[MODE_720_576_50]; >>> + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && >>> + (mode->vrefresh == 60)) >>> + my_mode = &decon_ext_porchs[MODE_1280_720_60]; >>> + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && >>> + (mode->vrefresh == 50)) >>> + my_mode = &decon_ext_porchs[MODE_1280_720_50]; >>> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >>> + (mode->vrefresh == 60)) >>> + my_mode = &decon_ext_porchs[MODE_1920_1080_60]; >>> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >>> + (mode->vrefresh == 50)) >>> + my_mode = &decon_ext_porchs[MODE_1920_1080_50]; >>> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >>> + (mode->vrefresh == 30)) >>> + my_mode = &decon_ext_porchs[MODE_1920_1080_30]; >>> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >>> + (mode->vrefresh == 25)) >>> + my_mode = &decon_ext_porchs[MODE_1920_1080_25]; >>> + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && >>> + (mode->vrefresh == 24)) >>> + my_mode = &decon_ext_porchs[MODE_1920_1080_24]; >>> + >>> + decon_ext_set_timing(mode, my_mode); >>> + } >>> + >> >> I guess these ugly ifs could be replaced by simple loop: >> for (i = 0; i < ARRAY_SIZE(decon_ext_porchs); ++i) { >> if (mode->hdisplay != decon_ext_porchs[i].hdisplay || ...) >> continue; >> decon_ext_set_timing(mode, &decon_ext_porchs[i]); >> break; >> } > Ok, Though the number of checks is the same, your code at least looks better. :) > >> Anyway I have doubts if commit should modify crtc.base.mode I guess >> better place for it can be in *_fixup callback. > Inki? What is your suggestion? Agree with Andrzej. The fixup callback would be a right place to find a valid mode. By the way, what if mode->hdisplay/vdisplay/vrefresh are not supported for all DECON_EXT modes? Thanks, Inki Dae
On 03/02/2015 11:57 AM, Ajay kumar wrote: > On Mon, Mar 2, 2015 at 1:38 PM, Andrzej Hajda <a.hajda@samsung.com> wrote: >> On 02/26/2015 04:24 PM, Ajay Kumar wrote: >>> * Modify DECON-INT driver to support DECON-EXT. >>> * Add a table of porch values needed to set timing registers of DECON-EXT. >>> * DECON-EXT supports only H/w Triggered COMMAND mode. >>> * DECON-EXT supports only one DMA window(window 1), so modify >>> all window management routines to support 2 windows of DECON-INT >>> and 1 window of DECON-EXT. >>> >>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >>> --- >>> .../devicetree/bindings/video/exynos7-decon.txt | 8 +- >>> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 229 ++++++++++++++++++-- >>> include/video/exynos7_decon.h | 13 ++ >>> 3 files changed, 230 insertions(+), 20 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/video/exynos7-decon.txt b/Documentation/devicetree/bindings/video/exynos7-decon.txt >>> index f5f9c8d..87350c0 100644 >>> --- a/Documentation/devicetree/bindings/video/exynos7-decon.txt >>> +++ b/Documentation/devicetree/bindings/video/exynos7-decon.txt >>> @@ -2,10 +2,14 @@ Device-Tree bindings for Samsung Exynos7 SoC display controller (DECON) >>> >>> DECON (Display and Enhancement Controller) is the Display Controller for the >>> Exynos7 series of SoCs which transfers the image data from a video memory >>> -buffer to an external LCD interface. >>> +buffer to an external LCD/HDMI interface. >>> + >>> +Exynos7 supports DECON-INT for generating video signals for LCD. >>> +Exynos7 supports DECON-EXT for generating video signals for HDMI. >>> >>> Required properties: >>> -- compatible: value should be "samsung,exynos7-decon"; >>> +- compatible: value should be "samsung,exynos7-decon-int" for DECON-INT; >>> + value should be "samsung,exynos7-decon-ext" for DECON-EXT; >>> >>> - reg: physical base address and length of the DECON registers set. >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c >>> index 63f02e2..9e2d083 100644 >>> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c >>> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c >>> @@ -41,6 +41,28 @@ >>> >>> #define WINDOWS_NR 2 >> Maybe it would be better to rename it to MAX_WINDOWS_NR > Ok. >>> +#define DECON_EXT_CH_MAPPING 0x432105 >> I am not familiar with decon, could you explain what exactly >> this mapping means and why is it fixed in the driver to this value, >> not default one. By the way my specs says bits 0-3 are reserverd >> and here it seems you are using them. > I reused the value from the code which hardware team shared with me. > It tries to map a input hardware channel(IDMA or VPP channel) onto > a hardware window in DECON. > Channel_N is mapped onto window_N in case of DECON-INT. > In case of DECON-EXT, Channel 0 should be mapped to window 1 of DECON-EXT. > So, basically for the changes I have made, I should ideally set only > bits [4:6] to 0x1, > and leave other bits untouched, though modifying other bits wouldn't > affect in anyway. >>> + >>> +enum decon_type { >>> + DECON_INT, >>> + DECON_EXT, >>> +} decon_type; >>> + >>> +struct decon_driver_data { >>> + enum decon_type decon_type; >>> + unsigned int nr_windows; >>> +}; >>> + >>> +static struct decon_driver_data exynos7_decon_int_driver_data = { >>> + .decon_type = DECON_INT, >> I wonder if it wouldn't be better to use different fields to describe >> each capability/property instead of decon_type. For example >> .display_type = EXYNOS_DISPLAY_TYPE_LCD, >> .map_channels = 0, > Ok, let me list down the differences between INT and EXT. > 1) Only h/w triggered command mode for DECON-EXT. > 2) Need to feed modified porch values(decon_ext_timings) > 3) Input channel to H/w window mapping(WINCHMAP) > 4) default_window - 0 for DECON-INT and 1 for DECON-EXT > > Out of the above differences, the first 3 can somehow be converted > to capability/property and embedded into driver_data. > But the 4th difference is bothering me. > I tried using something like start_window, end_window and tried to make > The code common for DECON-INT and DECON-EXT, and it just doesn't work. > ex: > start_window = 0, end_window = 1 for DECON-INT > start_window = 1, end_window = 1 for DECON-EXT > > When win_commit gets called for window 0 from crtc_commit/plane_commit: > Configure start_window(0 for DECON-INT and 1 for DECON-EXT) > > When win_commit is called from decon_apply, it is called for window 1 also. > That time win_commit can skip updating actual window 1. > How do I take care of this ambiguity? This case happens with > win_disable routine also! I think clear distinction where are we using hw windows and where sw windows should be enough. For example the loop iterating over all windows can look like: for (win = 0; win < drv_data->nr_windows; win++) { int hw_win = get_hw_win(ctx, win); val = readl(ctx->regs + WINCON(hw_win)); } Where get_hw_win can look like: static inline int get_hw_win(ctx, win) { return ctx->driver_data->skip_windows + win; } Regards Andrzej
On Wed, Mar 4, 2015 at 7:44 PM, Andrzej Hajda <a.hajda@samsung.com> wrote: > On 03/02/2015 11:57 AM, Ajay kumar wrote: >> On Mon, Mar 2, 2015 at 1:38 PM, Andrzej Hajda <a.hajda@samsung.com> wrote: >>> On 02/26/2015 04:24 PM, Ajay Kumar wrote: >>>> * Modify DECON-INT driver to support DECON-EXT. >>>> * Add a table of porch values needed to set timing registers of DECON-EXT. >>>> * DECON-EXT supports only H/w Triggered COMMAND mode. >>>> * DECON-EXT supports only one DMA window(window 1), so modify >>>> all window management routines to support 2 windows of DECON-INT >>>> and 1 window of DECON-EXT. >>>> >>>> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >>>> --- >>>> .../devicetree/bindings/video/exynos7-decon.txt | 8 +- >>>> drivers/gpu/drm/exynos/exynos7_drm_decon.c | 229 ++++++++++++++++++-- >>>> include/video/exynos7_decon.h | 13 ++ >>>> 3 files changed, 230 insertions(+), 20 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/video/exynos7-decon.txt b/Documentation/devicetree/bindings/video/exynos7-decon.txt >>>> index f5f9c8d..87350c0 100644 >>>> --- a/Documentation/devicetree/bindings/video/exynos7-decon.txt >>>> +++ b/Documentation/devicetree/bindings/video/exynos7-decon.txt >>>> @@ -2,10 +2,14 @@ Device-Tree bindings for Samsung Exynos7 SoC display controller (DECON) >>>> >>>> DECON (Display and Enhancement Controller) is the Display Controller for the >>>> Exynos7 series of SoCs which transfers the image data from a video memory >>>> -buffer to an external LCD interface. >>>> +buffer to an external LCD/HDMI interface. >>>> + >>>> +Exynos7 supports DECON-INT for generating video signals for LCD. >>>> +Exynos7 supports DECON-EXT for generating video signals for HDMI. >>>> >>>> Required properties: >>>> -- compatible: value should be "samsung,exynos7-decon"; >>>> +- compatible: value should be "samsung,exynos7-decon-int" for DECON-INT; >>>> + value should be "samsung,exynos7-decon-ext" for DECON-EXT; >>>> >>>> - reg: physical base address and length of the DECON registers set. >>>> >>>> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c >>>> index 63f02e2..9e2d083 100644 >>>> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c >>>> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c >>>> @@ -41,6 +41,28 @@ >>>> >>>> #define WINDOWS_NR 2 >>> Maybe it would be better to rename it to MAX_WINDOWS_NR >> Ok. >>>> +#define DECON_EXT_CH_MAPPING 0x432105 >>> I am not familiar with decon, could you explain what exactly >>> this mapping means and why is it fixed in the driver to this value, >>> not default one. By the way my specs says bits 0-3 are reserverd >>> and here it seems you are using them. >> I reused the value from the code which hardware team shared with me. >> It tries to map a input hardware channel(IDMA or VPP channel) onto >> a hardware window in DECON. >> Channel_N is mapped onto window_N in case of DECON-INT. >> In case of DECON-EXT, Channel 0 should be mapped to window 1 of DECON-EXT. >> So, basically for the changes I have made, I should ideally set only >> bits [4:6] to 0x1, >> and leave other bits untouched, though modifying other bits wouldn't >> affect in anyway. >>>> + >>>> +enum decon_type { >>>> + DECON_INT, >>>> + DECON_EXT, >>>> +} decon_type; >>>> + >>>> +struct decon_driver_data { >>>> + enum decon_type decon_type; >>>> + unsigned int nr_windows; >>>> +}; >>>> + >>>> +static struct decon_driver_data exynos7_decon_int_driver_data = { >>>> + .decon_type = DECON_INT, >>> I wonder if it wouldn't be better to use different fields to describe >>> each capability/property instead of decon_type. For example >>> .display_type = EXYNOS_DISPLAY_TYPE_LCD, >>> .map_channels = 0, >> Ok, let me list down the differences between INT and EXT. >> 1) Only h/w triggered command mode for DECON-EXT. >> 2) Need to feed modified porch values(decon_ext_timings) >> 3) Input channel to H/w window mapping(WINCHMAP) >> 4) default_window - 0 for DECON-INT and 1 for DECON-EXT >> >> Out of the above differences, the first 3 can somehow be converted >> to capability/property and embedded into driver_data. >> But the 4th difference is bothering me. >> I tried using something like start_window, end_window and tried to make >> The code common for DECON-INT and DECON-EXT, and it just doesn't work. >> ex: >> start_window = 0, end_window = 1 for DECON-INT >> start_window = 1, end_window = 1 for DECON-EXT >> >> When win_commit gets called for window 0 from crtc_commit/plane_commit: >> Configure start_window(0 for DECON-INT and 1 for DECON-EXT) >> >> When win_commit is called from decon_apply, it is called for window 1 also. >> That time win_commit can skip updating actual window 1. >> How do I take care of this ambiguity? This case happens with >> win_disable routine also! > > I think clear distinction where are we using hw windows and where sw > windows should be enough. > For example the loop iterating over all windows can look like: > > for (win = 0; win < drv_data->nr_windows; win++) { > int hw_win = get_hw_win(ctx, win); > > val = readl(ctx->regs + WINCON(hw_win)); > } > > Where get_hw_win can look like: > > static inline int get_hw_win(ctx, win) > { > return ctx->driver_data->skip_windows + win; > } OK, you just want it in a static wrapper. skip_windows = 0 for DECON-INT skip_windows = 1 for DECON-EXT Regards, Ajay
diff --git a/Documentation/devicetree/bindings/video/exynos7-decon.txt b/Documentation/devicetree/bindings/video/exynos7-decon.txt index f5f9c8d..87350c0 100644 --- a/Documentation/devicetree/bindings/video/exynos7-decon.txt +++ b/Documentation/devicetree/bindings/video/exynos7-decon.txt @@ -2,10 +2,14 @@ Device-Tree bindings for Samsung Exynos7 SoC display controller (DECON) DECON (Display and Enhancement Controller) is the Display Controller for the Exynos7 series of SoCs which transfers the image data from a video memory -buffer to an external LCD interface. +buffer to an external LCD/HDMI interface. + +Exynos7 supports DECON-INT for generating video signals for LCD. +Exynos7 supports DECON-EXT for generating video signals for HDMI. Required properties: -- compatible: value should be "samsung,exynos7-decon"; +- compatible: value should be "samsung,exynos7-decon-int" for DECON-INT; + value should be "samsung,exynos7-decon-ext" for DECON-EXT; - reg: physical base address and length of the DECON registers set. diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c index 63f02e2..9e2d083 100644 --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c @@ -41,6 +41,28 @@ #define WINDOWS_NR 2 +#define DECON_EXT_CH_MAPPING 0x432105 + +enum decon_type { + DECON_INT, + DECON_EXT, +} decon_type; + +struct decon_driver_data { + enum decon_type decon_type; + unsigned int nr_windows; +}; + +static struct decon_driver_data exynos7_decon_int_driver_data = { + .decon_type = DECON_INT, + .nr_windows = 2, +}; + +static struct decon_driver_data exynos7_decon_ext_driver_data = { + .decon_type = DECON_EXT, + .nr_windows = 1, +}; + struct decon_win_data { unsigned int ovl_x; unsigned int ovl_y; @@ -76,15 +98,28 @@ struct decon_context { atomic_t wait_vsync_event; struct exynos_drm_panel_info panel; + struct decon_driver_data *driver_data; struct exynos_drm_display *display; }; static const struct of_device_id decon_driver_dt_match[] = { - {.compatible = "samsung,exynos7-decon"}, + { .compatible = "samsung,exynos7-decon-int", + .data = &exynos7_decon_int_driver_data }, + { .compatible = "samsung,exynos7-decon-ext", + .data = &exynos7_decon_ext_driver_data }, {}, }; MODULE_DEVICE_TABLE(of, decon_driver_dt_match); +static inline struct decon_driver_data *drm_decon_get_driver_data( + struct platform_device *pdev) +{ + const struct of_device_id *of_id = + of_match_device(decon_driver_dt_match, &pdev->dev); + + return (struct decon_driver_data *)of_id->data; +} + static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc) { struct decon_context *ctx = crtc->ctx; @@ -106,13 +141,20 @@ static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc) static void decon_clear_channel(struct decon_context *ctx) { + struct decon_driver_data *drv_data = ctx->driver_data; int win, ch_enabled = 0; DRM_DEBUG_KMS("%s\n", __FILE__); /* Check if any channel is enabled. */ - for (win = 0; win < WINDOWS_NR; win++) { - u32 val = readl(ctx->regs + WINCON(win)); + for (win = 0; win < drv_data->nr_windows; win++) { + u32 val; + /* DECON EXT sw-hw window mapping */ + if (drv_data->decon_type == DECON_EXT) { + if (win == 0) + win = 1; + } + val = readl(ctx->regs + WINCON(win)); if (val & WINCONx_ENWIN) { val &= ~WINCONx_ENWIN; @@ -187,10 +229,74 @@ static bool decon_mode_fixup(struct exynos_drm_crtc *crtc, return true; } +struct decon_ext_videomode { + u32 vfront_porch; + u32 vback_porch; + u32 hfront_porch; + u32 hback_porch; + u32 vsync_len; + u32 hsync_len; + u32 hactive; + u32 vactive; + u32 refresh_rate; +}; + +enum DECON_EXT_MODES { + MODE_720_480_60, + MODE_720_576_50, + MODE_1280_720_60, + MODE_1280_720_50, + MODE_1920_1080_60, + MODE_1920_1080_50, + MODE_1920_1080_30, + MODE_1920_1080_25, + MODE_1920_1080_24, + MODE_3840_2160_30, + MODE_3840_2160_25, + MODE_3840_2160_24, + MODE_4096_2160_24, +}; + +static struct decon_ext_videomode decon_ext_porchs[] = { + /*vfp vbp hfp hbp vsa hsa xres yres fps */ + {1, 43, 10, 92, 1, 36, 720, 480, 60}, + {1, 47, 10, 92, 1, 42, 720, 576, 50}, + {1, 28, 10, 192, 1, 168, 1280, 720, 60}, + {1, 28, 10, 92, 1, 598, 1280, 720, 50}, + {1, 43, 10, 92, 1, 178, 1920, 1080, 60}, + {1, 43, 10, 92, 1, 618, 1920, 1080, 50}, + {1, 43, 10, 92, 1, 178, 1920, 1080, 30}, + {1, 43, 10, 92, 1, 618, 1920, 1080, 25}, + {1, 43, 10, 92, 1, 728, 1920, 1080, 24}, + {1, 88, 10, 458, 1, 92, 3840, 2160, 30}, + {1, 88, 10, 1338, 1, 92, 3840, 2160, 25}, + {1, 88, 10, 1558, 1, 92, 3840, 2160, 24}, + {1, 88, 10, 1302, 1, 92, 4096, 2160, 24}, +}; + +static void decon_ext_set_timing(struct drm_display_mode *mode, + struct decon_ext_videomode *ext_mode) +{ + mode->crtc_hdisplay = ext_mode->hactive; + mode->crtc_hsync_start = ext_mode->hactive + ext_mode->hfront_porch; + mode->crtc_hsync_end = ext_mode->hactive + ext_mode->hfront_porch + + ext_mode->hsync_len; + mode->crtc_htotal = ext_mode->hactive + ext_mode->hfront_porch + + ext_mode->hsync_len + ext_mode->hback_porch; + mode->crtc_vdisplay = ext_mode->vactive; + mode->crtc_vsync_start = ext_mode->vactive + ext_mode->vfront_porch; + mode->crtc_vsync_end = ext_mode->vactive + ext_mode->vfront_porch + + ext_mode->vsync_len; + mode->crtc_vtotal = ext_mode->vactive + ext_mode->vfront_porch + + ext_mode->vsync_len + ext_mode->vback_porch; +} + static void decon_commit(struct exynos_drm_crtc *crtc) { struct decon_context *ctx = crtc->ctx; + struct decon_driver_data *drv_data = ctx->driver_data; struct drm_display_mode *mode = &crtc->base.mode; + struct decon_ext_videomode *my_mode; u32 val, clkdiv; if (ctx->suspended) @@ -200,6 +306,38 @@ static void decon_commit(struct exynos_drm_crtc *crtc) if (mode->htotal == 0 || mode->vtotal == 0) return; + if (drv_data->decon_type == DECON_EXT) { + if ((mode->hdisplay == 720) && (mode->vdisplay == 480) && + (mode->vrefresh == 60)) + my_mode = &decon_ext_porchs[MODE_720_480_60]; + if ((mode->hdisplay == 720) && (mode->vdisplay == 576) && + (mode->vrefresh == 50)) + my_mode = &decon_ext_porchs[MODE_720_576_50]; + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && + (mode->vrefresh == 60)) + my_mode = &decon_ext_porchs[MODE_1280_720_60]; + if ((mode->hdisplay == 1280) && (mode->vdisplay == 720) && + (mode->vrefresh == 50)) + my_mode = &decon_ext_porchs[MODE_1280_720_50]; + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && + (mode->vrefresh == 60)) + my_mode = &decon_ext_porchs[MODE_1920_1080_60]; + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && + (mode->vrefresh == 50)) + my_mode = &decon_ext_porchs[MODE_1920_1080_50]; + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && + (mode->vrefresh == 30)) + my_mode = &decon_ext_porchs[MODE_1920_1080_30]; + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && + (mode->vrefresh == 25)) + my_mode = &decon_ext_porchs[MODE_1920_1080_25]; + if ((mode->hdisplay == 1920) && (mode->vdisplay == 1080) && + (mode->vrefresh == 24)) + my_mode = &decon_ext_porchs[MODE_1920_1080_24]; + + decon_ext_set_timing(mode, my_mode); + } + if (!ctx->i80_if) { int vsync_len, vbpd, vfpd, hsync_len, hbpd, hfpd; /* setup vertical timing values. */ @@ -301,6 +439,7 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, { struct decon_context *ctx = crtc->ctx; struct decon_win_data *win_data; + struct decon_driver_data *drv_data = ctx->driver_data; int win, padding; if (!plane) { @@ -312,12 +451,18 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, if (win == DEFAULT_ZPOS) win = ctx->default_win; - if (win < 0 || win >= WINDOWS_NR) + if (win < 0 || win >= drv_data->nr_windows) return; win_data = &ctx->win_data[win]; + /* DECON EXT sw-hw window mapping */ + if (drv_data->decon_type == DECON_EXT) { + if (win == 0) + win = 1; + } + padding = (plane->pitch / (plane->bpp >> 3)) - plane->fb_width; win_data->offset_x = plane->fb_x; win_data->offset_y = plane->fb_y; @@ -340,9 +485,9 @@ static void decon_win_mode_set(struct exynos_drm_crtc *crtc, plane->fb_width, plane->crtc_width); } -static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win) +static void decon_win_set_pixfmt(struct decon_context *ctx, unsigned int win, + struct decon_win_data *win_data) { - struct decon_win_data *win_data = &ctx->win_data[win]; unsigned long val; val = readl(ctx->regs + WINCON(win)); @@ -454,6 +599,7 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) struct decon_context *ctx = crtc->ctx; struct drm_display_mode *mode = &crtc->base.mode; struct decon_win_data *win_data; + struct decon_driver_data *drv_data = ctx->driver_data; int win = zpos; unsigned long val, alpha; unsigned int last_x; @@ -465,11 +611,17 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) if (win == DEFAULT_ZPOS) win = ctx->default_win; - if (win < 0 || win >= WINDOWS_NR) + if (win < 0 || win >= drv_data->nr_windows) return; win_data = &ctx->win_data[win]; + /* DECON EXT sw-hw window mapping */ + if (drv_data->decon_type == DECON_EXT) { + if (win == 0) + win = 1; + } + /* If suspended, enable this on resume */ if (ctx->suspended) { win_data->resume = true; @@ -546,7 +698,7 @@ static void decon_win_commit(struct exynos_drm_crtc *crtc, int zpos) writel(alpha, ctx->regs + VIDOSD_D(win)); - decon_win_set_pixfmt(ctx, win); + decon_win_set_pixfmt(ctx, win, win_data); /* hardware window 0 doesn't support color key. */ if (win != 0) @@ -572,17 +724,24 @@ static void decon_win_disable(struct exynos_drm_crtc *crtc, int zpos) { struct decon_context *ctx = crtc->ctx; struct decon_win_data *win_data; + struct decon_driver_data *drv_data = ctx->driver_data; int win = zpos; u32 val; if (win == DEFAULT_ZPOS) win = ctx->default_win; - if (win < 0 || win >= WINDOWS_NR) + if (win < 0 || win >= drv_data->nr_windows) return; win_data = &ctx->win_data[win]; + /* DECON EXT sw-hw window mapping */ + if (drv_data->decon_type == DECON_EXT) { + if (win == 0) + win = 1; + } + if (ctx->suspended) { /* do not resume this window*/ win_data->resume = false; @@ -609,10 +768,11 @@ static void decon_win_disable(struct exynos_drm_crtc *crtc, int zpos) static void decon_window_suspend(struct decon_context *ctx) { + struct decon_driver_data *drv_data = ctx->driver_data; struct decon_win_data *win_data; int i; - for (i = 0; i < WINDOWS_NR; i++) { + for (i = 0; i < drv_data->nr_windows; i++) { win_data = &ctx->win_data[i]; win_data->resume = win_data->enabled; if (win_data->enabled) @@ -622,10 +782,11 @@ static void decon_window_suspend(struct decon_context *ctx) static void decon_window_resume(struct decon_context *ctx) { + struct decon_driver_data *drv_data = ctx->driver_data; struct decon_win_data *win_data; int i; - for (i = 0; i < WINDOWS_NR; i++) { + for (i = 0; i < drv_data->nr_windows; i++) { win_data = &ctx->win_data[i]; win_data->enabled = win_data->resume; win_data->resume = false; @@ -634,10 +795,11 @@ static void decon_window_resume(struct decon_context *ctx) static void decon_apply(struct decon_context *ctx) { + struct decon_driver_data *drv_data = ctx->driver_data; struct decon_win_data *win_data; int i; - for (i = 0; i < WINDOWS_NR; i++) { + for (i = 0; i < drv_data->nr_windows; i++) { win_data = &ctx->win_data[i]; if (win_data->enabled) decon_win_commit(ctx->crtc, i); @@ -650,6 +812,7 @@ static void decon_apply(struct decon_context *ctx) static void decon_init(struct decon_context *ctx) { + struct decon_driver_data *drv_data = ctx->driver_data; u32 val; writel(VIDCON0_SWRESET, ctx->regs + VIDCON0); @@ -657,6 +820,14 @@ static void decon_init(struct decon_context *ctx) val = VIDOUTCON0_DISP_IF_0_ON; if (!ctx->i80_if) val |= VIDOUTCON0_RGBIF; + + if (drv_data->decon_type == DECON_EXT) { + writel(TRIGCON_HWTRIG_AUTO_MASK | + TRIGCON_HWTRIGMASK_DISPIF0 | + TRIGCON_HWTRIGEN_I80_RGB, ctx->regs + TRIGCON); + val |= VIDOUTCON0_TV_MODE | VIDOUTCON0_I80IF; + } + writel(val, ctx->regs + VIDOUTCON0); writel(VCLKCON0_CLKVALUP | VCLKCON0_VCLKFREE, ctx->regs + VCLKCON0); @@ -667,6 +838,7 @@ static void decon_init(struct decon_context *ctx) static int decon_poweron(struct decon_context *ctx) { + struct decon_driver_data *drv_data = ctx->driver_data; int ret; if (!ctx->suspended) @@ -702,6 +874,9 @@ static int decon_poweron(struct decon_context *ctx) decon_init(ctx); + if (drv_data->decon_type == DECON_EXT) + writel(DECON_EXT_CH_MAPPING, ctx->regs + WINCHMAP0); + /* if vblank was enabled status, enable it again. */ if (test_and_clear_bit(0, &ctx->irq_flags)) { ret = decon_enable_vblank(ctx->crtc); @@ -817,6 +992,7 @@ out: static int decon_bind(struct device *dev, struct device *master, void *data) { struct decon_context *ctx = dev_get_drvdata(dev); + struct decon_driver_data *drv_data = ctx->driver_data; struct drm_device *drm_dev = data; int ret; @@ -826,16 +1002,23 @@ static int decon_bind(struct device *dev, struct device *master, void *data) return ret; } - ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, + if (drv_data->decon_type == DECON_INT) + ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, EXYNOS_DISPLAY_TYPE_LCD, &decon_crtc_ops, ctx); + else + ctx->crtc = exynos_drm_crtc_create(drm_dev, ctx->pipe, + EXYNOS_DISPLAY_TYPE_HDMI, + &decon_crtc_ops, ctx); + if (IS_ERR(ctx->crtc)) { decon_ctx_remove(ctx); return PTR_ERR(ctx->crtc); } - if (ctx->display) - exynos_drm_create_enc_conn(drm_dev, ctx->display); + if (drv_data->decon_type == DECON_INT) + if (ctx->display) + exynos_drm_create_enc_conn(drm_dev, ctx->display); return 0; @@ -848,8 +1031,9 @@ static void decon_unbind(struct device *dev, struct device *master, decon_dpms(ctx->crtc, DRM_MODE_DPMS_OFF); - if (ctx->display) - exynos_dpi_remove(ctx->display); + if (ctx->driver_data->decon_type == DECON_INT) + if (ctx->display) + exynos_dpi_remove(ctx->display); decon_ctx_remove(ctx); } @@ -862,11 +1046,14 @@ static const struct component_ops decon_component_ops = { static int decon_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; + struct decon_driver_data *drv_data; struct decon_context *ctx; struct device_node *i80_if_timings; struct resource *res; int ret; + drv_data = drm_decon_get_driver_data(pdev); + if (!dev->of_node) return -ENODEV; @@ -874,11 +1061,17 @@ static int decon_probe(struct platform_device *pdev) if (!ctx) return -ENOMEM; - ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, + if (drv_data->decon_type == DECON_INT) + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, EXYNOS_DISPLAY_TYPE_LCD); + else + ret = exynos_drm_component_add(dev, EXYNOS_DEVICE_TYPE_CRTC, + EXYNOS_DISPLAY_TYPE_HDMI); + if (ret) return ret; + ctx->driver_data = drv_data; ctx->dev = dev; ctx->suspended = true; diff --git a/include/video/exynos7_decon.h b/include/video/exynos7_decon.h index a62b11b..d02320b 100644 --- a/include/video/exynos7_decon.h +++ b/include/video/exynos7_decon.h @@ -20,6 +20,7 @@ /* VIDOUTCON0 */ #define VIDOUTCON0 0x4 +#define VIDOUTCON0_TV_MODE (0x1 << 26) #define VIDOUTCON0_DUAL_MASK (0x3 << 24) #define VIDOUTCON0_DUAL_ON (0x3 << 24) #define VIDOUTCON0_DISP_IF_1_ON (0x2 << 24) @@ -52,6 +53,9 @@ #define SHADOWCON_WINx_PROTECT(_win) (1 << (10 + (_win))) +/* WINCHMAP0 */ +#define WINCHMAP0 0x40 + /* WINCONx */ #define WINCON(_win) (0x50 + ((_win) * 4)) @@ -328,6 +332,15 @@ /* LINECNT OP THRSHOLD*/ #define LINECNT_OP_THRESHOLD 0x630 +/* TRIGCON */ +#define TRIGCON 0x06B0 +#define TRIGCON_HWTRIG_AUTO_MASK (1 << 6) +#define TRIGCON_HWTRIGMASK_DISPIF1 (1 << 5) +#define TRIGCON_HWTRIGMASK_DISPIF0 (1 << 4) +#define TRIGCON_HWTRIGEN_I80_RGB (1 << 3) +#define TRIGCON_SWTRIGCMD_I80_RGB (1 << 1) +#define TRIGCON_SWTRIGEN_I80_RGB (1 << 0) + /* CRCCTRL */ #define CRCCTRL 0x6C8 #define CRCCTRL_CRCCLKEN (0x1 << 2)
* Modify DECON-INT driver to support DECON-EXT. * Add a table of porch values needed to set timing registers of DECON-EXT. * DECON-EXT supports only H/w Triggered COMMAND mode. * DECON-EXT supports only one DMA window(window 1), so modify all window management routines to support 2 windows of DECON-INT and 1 window of DECON-EXT. Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> --- .../devicetree/bindings/video/exynos7-decon.txt | 8 +- drivers/gpu/drm/exynos/exynos7_drm_decon.c | 229 ++++++++++++++++++-- include/video/exynos7_decon.h | 13 ++ 3 files changed, 230 insertions(+), 20 deletions(-)