| Message ID | 1403793398-31593-1-git-send-email-ajaykumar.rs@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thursday, June 26, 2014 11:37 PM, Ajay Kumar wrote: > > Add the missing setting for DP CLKCON register. > > This register is present on Exynos5 based FIMD controllers, > and needs to be used if we are using DP. > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> It looks better than V1 patch. Reviewed-by: Jingoo Han <jg1.han@samsung.com> Best regards, Jingoo Han > --- > Changes since V1: > - Remove usage of driver_data to configure DP CLKCON register > drivers/gpu/drm/exynos/exynos_dp_core.c | 2 ++ > drivers/gpu/drm/exynos/exynos_drm_drv.h | 8 ++++++++ > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 +++++ > include/video/samsung_fimd.h | 4 ++++ > 4 files changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c > index 2e77a15..d8868f3 100644 > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c > @@ -1336,6 +1336,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) > > platform_set_drvdata(pdev, &exynos_dp_display); > > + exynos_fimd_output_type = EXYNOS_FIMD_OUTPUT_DP; > + > return exynos_drm_create_enc_conn(drm_dev, &exynos_dp_display); > } > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h > index 36535f3..1089744 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h > @@ -60,6 +60,12 @@ enum exynos_drm_output_type { > EXYNOS_DISPLAY_TYPE_VIDI, > }; > > +enum exynos_fimd_output_type { > + EXYNOS_FIMD_OUTPUT_MIPI, > + EXYNOS_FIMD_OUTPUT_DPI, > + EXYNOS_FIMD_OUTPUT_DP, > +}; > + > /* > * Exynos drm common overlay structure. > * > @@ -380,4 +386,6 @@ extern struct platform_driver fimc_driver; > extern struct platform_driver rotator_driver; > extern struct platform_driver gsc_driver; > extern struct platform_driver ipp_driver; > + > +extern enum exynos_fimd_output_type exynos_fimd_output_type; > #endif > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index bb45ab2..a46a9c4 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -90,6 +90,8 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { > .has_shadowcon = 1, > }; > > +enum exynos_fimd_output_type exynos_fimd_output_type; > + > struct fimd_win_data { > unsigned int offset_x; > unsigned int offset_y; > @@ -331,6 +333,9 @@ static void fimd_commit(struct exynos_drm_manager *mgr) > if (clkdiv > 1) > val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; > > + if (exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) > + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); > + > writel(val, ctx->regs + VIDCON0); > } > > diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h > index b039320..d8f4b0b 100644 > --- a/include/video/samsung_fimd.h > +++ b/include/video/samsung_fimd.h > @@ -435,6 +435,10 @@ > #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) > #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0) > > +/* Video clock enable for DP */ > +#define DP_CLKCON 0x27C > +#define DP_CLK_ENABLE 0x2 > + > /* Notes on per-window bpp settings > * > * Value Win0 Win1 Win2 Win3 Win 4 > -- > 1.8.3.2
On 2014? 06? 26? 23:36, Ajay Kumar wrote: > Add the missing setting for DP CLKCON register. > > This register is present on Exynos5 based FIMD controllers, > and needs to be used if we are using DP. > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > --- > Changes since V1: > - Remove usage of driver_data to configure DP CLKCON register > drivers/gpu/drm/exynos/exynos_dp_core.c | 2 ++ > drivers/gpu/drm/exynos/exynos_drm_drv.h | 8 ++++++++ > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 +++++ > include/video/samsung_fimd.h | 4 ++++ > 4 files changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c > index 2e77a15..d8868f3 100644 > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c > @@ -1336,6 +1336,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) > > platform_set_drvdata(pdev, &exynos_dp_display); > > + exynos_fimd_output_type = EXYNOS_FIMD_OUTPUT_DP; > + > return exynos_drm_create_enc_conn(drm_dev, &exynos_dp_display); > } > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h > index 36535f3..1089744 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h > @@ -60,6 +60,12 @@ enum exynos_drm_output_type { > EXYNOS_DISPLAY_TYPE_VIDI, > }; > > +enum exynos_fimd_output_type { > + EXYNOS_FIMD_OUTPUT_MIPI, > + EXYNOS_FIMD_OUTPUT_DPI, > + EXYNOS_FIMD_OUTPUT_DP, > +}; > + > /* > * Exynos drm common overlay structure. > * > @@ -380,4 +386,6 @@ extern struct platform_driver fimc_driver; > extern struct platform_driver rotator_driver; > extern struct platform_driver gsc_driver; > extern struct platform_driver ipp_driver; > + > +extern enum exynos_fimd_output_type exynos_fimd_output_type; > #endif > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index bb45ab2..a46a9c4 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -90,6 +90,8 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { > .has_shadowcon = 1, > }; > > +enum exynos_fimd_output_type exynos_fimd_output_type; > + > struct fimd_win_data { > unsigned int offset_x; > unsigned int offset_y; > @@ -331,6 +333,9 @@ static void fimd_commit(struct exynos_drm_manager *mgr) > if (clkdiv > 1) > val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; > > + if (exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) > + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); You are passing output type directly from dp driver to fimd driver using extern variable. This is really not good. Is that really only way for it? One is, it's to add a dt property in fimd node and use it; ex. samsung,output_type. Whether fimd output should be DPI, DSI, or eDP would be dependent on Machine. Other is, crtc driver is to get output type though encoder callback. At this moment, crtc driver already knows encoder/connector driver. So I think you can use a callback of encoder to get output type like below, exynos_display_ops { ... ...get_output(struct drm_crtc *crtc); }; at exynos_drm_encoder.c, ... exynos_drm_get_output_type(struct drm_crtc *crtc) { ... return display_ops->get_output(...); } at fimd_commit function, ... ctx->output_type = exynos_drm_get_output_type(mgr->crtc); I think first way is more simple and reasonable. One more thing, does FIMD ip of Exynos4 series have DP_CLKCON register? No, they have no any DP_CLKCON register so you would need to consider other SoC without DP_CLKCON. For this, a proper volunteer would be fimd_driver_data. Thanks, Inki Dae > + > writel(val, ctx->regs + VIDCON0); > } > > diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h > index b039320..d8f4b0b 100644 > --- a/include/video/samsung_fimd.h > +++ b/include/video/samsung_fimd.h > @@ -435,6 +435,10 @@ > #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) > #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0) > > +/* Video clock enable for DP */ > +#define DP_CLKCON 0x27C > +#define DP_CLK_ENABLE 0x2 > + > /* Notes on per-window bpp settings > * > * Value Win0 Win1 Win2 Win3 Win 4 >
Am 26.06.2014 16:36, schrieb Ajay Kumar: > Add the missing setting for DP CLKCON register. > > This register is present on Exynos5 based FIMD controllers, > and needs to be used if we are using DP. > > Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> > --- > Changes since V1: > - Remove usage of driver_data to configure DP CLKCON register > drivers/gpu/drm/exynos/exynos_dp_core.c | 2 ++ > drivers/gpu/drm/exynos/exynos_drm_drv.h | 8 ++++++++ > drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 +++++ > include/video/samsung_fimd.h | 4 ++++ > 4 files changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c > index 2e77a15..d8868f3 100644 > --- a/drivers/gpu/drm/exynos/exynos_dp_core.c > +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c > @@ -1336,6 +1336,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) > > platform_set_drvdata(pdev, &exynos_dp_display); > > + exynos_fimd_output_type = EXYNOS_FIMD_OUTPUT_DP; > + > return exynos_drm_create_enc_conn(drm_dev, &exynos_dp_display); > } > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h > index 36535f3..1089744 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h > +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h > @@ -60,6 +60,12 @@ enum exynos_drm_output_type { > EXYNOS_DISPLAY_TYPE_VIDI, > }; > > +enum exynos_fimd_output_type { > + EXYNOS_FIMD_OUTPUT_MIPI, > + EXYNOS_FIMD_OUTPUT_DPI, > + EXYNOS_FIMD_OUTPUT_DP, > +}; > + > /* > * Exynos drm common overlay structure. > * > @@ -380,4 +386,6 @@ extern struct platform_driver fimc_driver; > extern struct platform_driver rotator_driver; > extern struct platform_driver gsc_driver; > extern struct platform_driver ipp_driver; > + > +extern enum exynos_fimd_output_type exynos_fimd_output_type; > #endif > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > index bb45ab2..a46a9c4 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c > @@ -90,6 +90,8 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { > .has_shadowcon = 1, > }; > > +enum exynos_fimd_output_type exynos_fimd_output_type; This is a non-static variable - doesn't that mean it's only ever initialized to DP above and uninitialized in C99 otherwise? Unless I'm missing something, the if below may trigger at random. Regards, Andreas > + > struct fimd_win_data { > unsigned int offset_x; > unsigned int offset_y; > @@ -331,6 +333,9 @@ static void fimd_commit(struct exynos_drm_manager *mgr) > if (clkdiv > 1) > val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; > > + if (exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) > + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); > + > writel(val, ctx->regs + VIDCON0); > } > > diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h > index b039320..d8f4b0b 100644 > --- a/include/video/samsung_fimd.h > +++ b/include/video/samsung_fimd.h > @@ -435,6 +435,10 @@ > #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) > #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0) > > +/* Video clock enable for DP */ > +#define DP_CLKCON 0x27C > +#define DP_CLK_ENABLE 0x2 > + > /* Notes on per-window bpp settings > * > * Value Win0 Win1 Win2 Win3 Win 4
Hi Inki, Thanks for the review. On Fri, Jun 27, 2014 at 1:04 PM, Inki Dae <inki.dae@samsung.com> wrote: > On 2014? 06? 26? 23:36, Ajay Kumar wrote: >> Add the missing setting for DP CLKCON register. >> >> This register is present on Exynos5 based FIMD controllers, >> and needs to be used if we are using DP. >> >> Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> >> --- >> Changes since V1: >> - Remove usage of driver_data to configure DP CLKCON register >> drivers/gpu/drm/exynos/exynos_dp_core.c | 2 ++ >> drivers/gpu/drm/exynos/exynos_drm_drv.h | 8 ++++++++ >> drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 +++++ >> include/video/samsung_fimd.h | 4 ++++ >> 4 files changed, 19 insertions(+) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c >> index 2e77a15..d8868f3 100644 >> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c >> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c >> @@ -1336,6 +1336,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) >> >> platform_set_drvdata(pdev, &exynos_dp_display); >> >> + exynos_fimd_output_type = EXYNOS_FIMD_OUTPUT_DP; >> + >> return exynos_drm_create_enc_conn(drm_dev, &exynos_dp_display); >> } >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h >> index 36535f3..1089744 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h >> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h >> @@ -60,6 +60,12 @@ enum exynos_drm_output_type { >> EXYNOS_DISPLAY_TYPE_VIDI, >> }; >> >> +enum exynos_fimd_output_type { >> + EXYNOS_FIMD_OUTPUT_MIPI, >> + EXYNOS_FIMD_OUTPUT_DPI, >> + EXYNOS_FIMD_OUTPUT_DP, >> +}; >> + >> /* >> * Exynos drm common overlay structure. >> * >> @@ -380,4 +386,6 @@ extern struct platform_driver fimc_driver; >> extern struct platform_driver rotator_driver; >> extern struct platform_driver gsc_driver; >> extern struct platform_driver ipp_driver; >> + >> +extern enum exynos_fimd_output_type exynos_fimd_output_type; >> #endif >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> index bb45ab2..a46a9c4 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c >> @@ -90,6 +90,8 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { >> .has_shadowcon = 1, >> }; >> >> +enum exynos_fimd_output_type exynos_fimd_output_type; >> + >> struct fimd_win_data { >> unsigned int offset_x; >> unsigned int offset_y; >> @@ -331,6 +333,9 @@ static void fimd_commit(struct exynos_drm_manager *mgr) >> if (clkdiv > 1) >> val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; >> >> + if (exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) >> + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); > > > You are passing output type directly from dp driver to fimd driver using > extern variable. This is really not good. Is that really only way for it? > > One is, > it's to add a dt property in fimd node and use it; ex. > samsung,output_type. Whether fimd output should be DPI, DSI, or eDP > would be dependent on Machine. > > Other is, > crtc driver is to get output type though encoder callback. At this > moment, crtc driver already knows encoder/connector driver. So I think > you can use a callback of encoder to get output type like below, > > exynos_display_ops { > ... > ...get_output(struct drm_crtc *crtc); > }; > > at exynos_drm_encoder.c, > ... exynos_drm_get_output_type(struct drm_crtc *crtc) > { > ... > return display_ops->get_output(...); > } > > at fimd_commit function, > ... > ctx->output_type = exynos_drm_get_output_type(mgr->crtc); > > > I think first way is more simple and reasonable. > > One more thing, does FIMD ip of Exynos4 series have DP_CLKCON register? > No, they have no any DP_CLKCON register so you would need to consider > other SoC without DP_CLKCON. For this, a proper volunteer would be > fimd_driver_data. Right, I should also consider putting DP CLKCON register inside driver_data. And, I will use the DT property you mentioned. Will send a patchset ASAP. Regards, Ajay Kumar > Thanks, > Inki Dae > > >> + >> writel(val, ctx->regs + VIDCON0); >> } >> >> diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h >> index b039320..d8f4b0b 100644 >> --- a/include/video/samsung_fimd.h >> +++ b/include/video/samsung_fimd.h >> @@ -435,6 +435,10 @@ >> #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) >> #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0) >> >> +/* Video clock enable for DP */ >> +#define DP_CLKCON 0x27C >> +#define DP_CLK_ENABLE 0x2 >> + >> /* Notes on per-window bpp settings >> * >> * Value Win0 Win1 Win2 Win3 Win 4 >> >
diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c index 2e77a15..d8868f3 100644 --- a/drivers/gpu/drm/exynos/exynos_dp_core.c +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c @@ -1336,6 +1336,8 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data) platform_set_drvdata(pdev, &exynos_dp_display); + exynos_fimd_output_type = EXYNOS_FIMD_OUTPUT_DP; + return exynos_drm_create_enc_conn(drm_dev, &exynos_dp_display); } diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h index 36535f3..1089744 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h @@ -60,6 +60,12 @@ enum exynos_drm_output_type { EXYNOS_DISPLAY_TYPE_VIDI, }; +enum exynos_fimd_output_type { + EXYNOS_FIMD_OUTPUT_MIPI, + EXYNOS_FIMD_OUTPUT_DPI, + EXYNOS_FIMD_OUTPUT_DP, +}; + /* * Exynos drm common overlay structure. * @@ -380,4 +386,6 @@ extern struct platform_driver fimc_driver; extern struct platform_driver rotator_driver; extern struct platform_driver gsc_driver; extern struct platform_driver ipp_driver; + +extern enum exynos_fimd_output_type exynos_fimd_output_type; #endif diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index bb45ab2..a46a9c4 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c @@ -90,6 +90,8 @@ static struct fimd_driver_data exynos5_fimd_driver_data = { .has_shadowcon = 1, }; +enum exynos_fimd_output_type exynos_fimd_output_type; + struct fimd_win_data { unsigned int offset_x; unsigned int offset_y; @@ -331,6 +333,9 @@ static void fimd_commit(struct exynos_drm_manager *mgr) if (clkdiv > 1) val |= VIDCON0_CLKVAL_F(clkdiv - 1) | VIDCON0_CLKDIR; + if (exynos_fimd_output_type == EXYNOS_FIMD_OUTPUT_DP) + writel(DP_CLK_ENABLE, ctx->regs + DP_CLKCON); + writel(val, ctx->regs + VIDCON0); } diff --git a/include/video/samsung_fimd.h b/include/video/samsung_fimd.h index b039320..d8f4b0b 100644 --- a/include/video/samsung_fimd.h +++ b/include/video/samsung_fimd.h @@ -435,6 +435,10 @@ #define BLENDCON_NEW_8BIT_ALPHA_VALUE (1 << 0) #define BLENDCON_NEW_4BIT_ALPHA_VALUE (0 << 0) +/* Video clock enable for DP */ +#define DP_CLKCON 0x27C +#define DP_CLK_ENABLE 0x2 + /* Notes on per-window bpp settings * * Value Win0 Win1 Win2 Win3 Win 4
Add the missing setting for DP CLKCON register. This register is present on Exynos5 based FIMD controllers, and needs to be used if we are using DP. Signed-off-by: Ajay Kumar <ajaykumar.rs@samsung.com> --- Changes since V1: - Remove usage of driver_data to configure DP CLKCON register drivers/gpu/drm/exynos/exynos_dp_core.c | 2 ++ drivers/gpu/drm/exynos/exynos_drm_drv.h | 8 ++++++++ drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 +++++ include/video/samsung_fimd.h | 4 ++++ 4 files changed, 19 insertions(+)