[PATCH v3 2/2] drm/mediatek: set mt8173 dithering function
Hi, Bibby:
On Thu, 2016-07-21 at 11:21 +0800, Bibby Hsieh wrote:
> Hi, CK
>
> I'm appreciate your comments.
>
>
[snip...]
> > >
> > > @@ -469,7 +484,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct
> > > mtk_ddp_comp *ovl)
> > > if (state->pending_config) {
> > > mtk_ddp_comp_config(ovl, state->pending_width,
> > > state->pending_height,
> > > - state->pending_vrefresh);
> > > + state->pending_vrefresh, 0);
> >
> > Why set bpc as 0 here? Maybe you have a assumption that OVL don't care
> > the bpc parameter. If one day OVL care it and we do not review here, the
> > bugs happen.
> >
> Pass 0 means don't care, I will modify mtk_od_config() and
> mtk_gamma_config() to filter this condition.
Using 0 as 'don't care' looks tricky. Please add comments to describe
this.
> > >
> > > state->pending_config = false;
> > > }
Regards,
CK
[PATCH v3 2/2] drm/mediatek: set mt8173 dithering function
Hi, CK
I'm appreciate your comments.
On Mon, 2016-07-18 at 10:33 +0800, CK Hu wrote:
> Hi, Bibby:
>
> Some comments inline.
>
> On Thu, 2016-07-07 at 15:37 +0800, Bibby Hsieh wrote:
> > Some panels only accept bpc (bit per color) 6-bit.
> > But, the default bpc in mt8173 display data path is 8-bit.
> > If we didn't enable dithering function to convert bpc,
> > display cannot show the smooth grayscale image.
> >
> > In mt8173, the dithering function in OD (OverDrive) and
> > GAMMA module, we have to config them with
> > connector->display_mode.bpc when CRTC initial.
> >
> > 1. Clear the default value at *_DITHER_5 and *_DITHER_7 register.
> > 2. Calculate the LSB_ERR_SHIFT bits and ADD_LSHIFT bits two values.
> > i.e. Input bpc of OD is 10 bits, we assume the bpc of panel is 6-bit,
> > so, we need to set 4-bit to LSB_ERR_SHIFT and ADD_LSHIFT bits respectively.
> > 3. Then, set the OD or GAMMA to dithering mode depends on path-1 or path-2.
> >
> > Signed-off-by: Bibby Hsieh
> > ---
> > drivers/gpu/drm/mediatek/mtk_disp_ovl.c |3 +-
> > drivers/gpu/drm/mediatek/mtk_disp_rdma.c|3 +-
> > drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 21 ++--
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 77
> > +++
> > drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |6 +--
> > 5 files changed, 92 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 8f62671f..019b7ca 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -103,7 +103,8 @@ static void mtk_ovl_stop(struct mtk_ddp_comp *comp)
> > }
> >
> > static void mtk_ovl_config(struct mtk_ddp_comp *comp, unsigned int w,
> > - unsigned int h, unsigned int vrefresh)
> > + unsigned int h, unsigned int vrefresh,
> > + unsigned int bpc)
> > {
> > if (w != 0 && h != 0)
> > writel_relaxed(h << 16 | w, comp->regs + DISP_REG_OVL_ROI_SIZE);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > index 5fb80cb..0df05f9 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> > @@ -106,7 +106,8 @@ static void mtk_rdma_stop(struct mtk_ddp_comp *comp)
> > }
> >
> > static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
> > - unsigned int height, unsigned int vrefresh)
> > + unsigned int height, unsigned int vrefresh,
> > + unsigned int bpc)
> > {
> > unsigned int threshold;
> > unsigned int reg;
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index ee219bb..18c9d0d 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -222,7 +222,9 @@ static void mtk_crtc_ddp_clk_disable(struct
> > mtk_drm_crtc *mtk_crtc)
> > static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
> > {
> > struct drm_crtc *crtc = _crtc->base;
> > - unsigned int width, height, vrefresh;
> > + struct drm_connector *connector;
> > + struct drm_encoder *encoder;
> > + unsigned int width, height, vrefresh, bpc = 0;
> > int ret;
> > int i;
> >
> > @@ -234,6 +236,19 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> > *mtk_crtc)
> > height = crtc->state->adjusted_mode.vdisplay;
> > vrefresh = crtc->state->adjusted_mode.vrefresh;
> >
> > + drm_for_each_encoder(encoder, crtc->dev) {
> > + if (encoder->crtc != crtc)
> > + continue;
> > +
> > + drm_for_each_connector(connector, crtc->dev) {
> > + if (connector->encoder != encoder)
> > + continue;
> > + if (connector->display_info.bpc >= 3 &&
>
> I think you should left this HW constrain in mtk_od_config() and
> mtk_gamma_config(). Here just calculate the expected bpc.
>
Ok, I will do that.
> > + (bpc == 0 || bpc > connector->display_info.bpc))
>
> I think bpc should be initialized as HW default bpc and you can remove
> this condiction 'bpc == 0' because we should set bpc to HW default bpc
> while all connnector bpc is greater than HW default bpc.
>
Ok, will do.
> > + bpc = connector->display_info.bpc;
> > + }
> > + }
> > +
> > ret = pm_runtime_get_sync(crtc->dev->dev);
> > if (ret < 0) {
> > DRM_ERROR("Failed to enable power domain: %d\n", ret);
> > @@ -266,7 +281,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> > *mtk_crtc)
> > for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> > struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[i];
> >
> > - mtk_ddp_comp_config(comp, width, height, vrefresh);
> > +
[PATCH v3 2/2] drm/mediatek: set mt8173 dithering function
Hi, Bibby:
Some comments inline.
On Thu, 2016-07-07 at 15:37 +0800, Bibby Hsieh wrote:
> Some panels only accept bpc (bit per color) 6-bit.
> But, the default bpc in mt8173 display data path is 8-bit.
> If we didn't enable dithering function to convert bpc,
> display cannot show the smooth grayscale image.
>
> In mt8173, the dithering function in OD (OverDrive) and
> GAMMA module, we have to config them with
> connector->display_mode.bpc when CRTC initial.
>
> 1. Clear the default value at *_DITHER_5 and *_DITHER_7 register.
> 2. Calculate the LSB_ERR_SHIFT bits and ADD_LSHIFT bits two values.
> i.e. Input bpc of OD is 10 bits, we assume the bpc of panel is 6-bit,
> so, we need to set 4-bit to LSB_ERR_SHIFT and ADD_LSHIFT bits respectively.
> 3. Then, set the OD or GAMMA to dithering mode depends on path-1 or path-2.
>
> Signed-off-by: Bibby Hsieh
> ---
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c |3 +-
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c|3 +-
> drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 21 ++--
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 77
> +++
> drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |6 +--
> 5 files changed, 92 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 8f62671f..019b7ca 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -103,7 +103,8 @@ static void mtk_ovl_stop(struct mtk_ddp_comp *comp)
> }
>
> static void mtk_ovl_config(struct mtk_ddp_comp *comp, unsigned int w,
> -unsigned int h, unsigned int vrefresh)
> +unsigned int h, unsigned int vrefresh,
> +unsigned int bpc)
> {
> if (w != 0 && h != 0)
> writel_relaxed(h << 16 | w, comp->regs + DISP_REG_OVL_ROI_SIZE);
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index 5fb80cb..0df05f9 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -106,7 +106,8 @@ static void mtk_rdma_stop(struct mtk_ddp_comp *comp)
> }
>
> static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
> - unsigned int height, unsigned int vrefresh)
> + unsigned int height, unsigned int vrefresh,
> + unsigned int bpc)
> {
> unsigned int threshold;
> unsigned int reg;
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> index ee219bb..18c9d0d 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> @@ -222,7 +222,9 @@ static void mtk_crtc_ddp_clk_disable(struct mtk_drm_crtc
> *mtk_crtc)
> static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
> {
> struct drm_crtc *crtc = _crtc->base;
> - unsigned int width, height, vrefresh;
> + struct drm_connector *connector;
> + struct drm_encoder *encoder;
> + unsigned int width, height, vrefresh, bpc = 0;
> int ret;
> int i;
>
> @@ -234,6 +236,19 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> *mtk_crtc)
> height = crtc->state->adjusted_mode.vdisplay;
> vrefresh = crtc->state->adjusted_mode.vrefresh;
>
> + drm_for_each_encoder(encoder, crtc->dev) {
> + if (encoder->crtc != crtc)
> + continue;
> +
> + drm_for_each_connector(connector, crtc->dev) {
> + if (connector->encoder != encoder)
> + continue;
> + if (connector->display_info.bpc >= 3 &&
I think you should left this HW constrain in mtk_od_config() and
mtk_gamma_config(). Here just calculate the expected bpc.
> + (bpc == 0 || bpc > connector->display_info.bpc))
I think bpc should be initialized as HW default bpc and you can remove
this condiction 'bpc == 0' because we should set bpc to HW default bpc
while all connnector bpc is greater than HW default bpc.
> + bpc = connector->display_info.bpc;
> + }
> + }
> +
> ret = pm_runtime_get_sync(crtc->dev->dev);
> if (ret < 0) {
> DRM_ERROR("Failed to enable power domain: %d\n", ret);
> @@ -266,7 +281,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
> *mtk_crtc)
> for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
> struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[i];
>
> - mtk_ddp_comp_config(comp, width, height, vrefresh);
> + mtk_ddp_comp_config(comp, width, height, vrefresh, bpc);
> mtk_ddp_comp_start(comp);
> }
>
> @@ -469,7 +484,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct
> mtk_ddp_comp *ovl)
> if (state->pending_config) {
>
[PATCH v3 2/2] drm/mediatek: set mt8173 dithering function
Some panels only accept bpc (bit per color) 6-bit.
But, the default bpc in mt8173 display data path is 8-bit.
If we didn't enable dithering function to convert bpc,
display cannot show the smooth grayscale image.
In mt8173, the dithering function in OD (OverDrive) and
GAMMA module, we have to config them with
connector->display_mode.bpc when CRTC initial.
1. Clear the default value at *_DITHER_5 and *_DITHER_7 register.
2. Calculate the LSB_ERR_SHIFT bits and ADD_LSHIFT bits two values.
i.e. Input bpc of OD is 10 bits, we assume the bpc of panel is 6-bit,
so, we need to set 4-bit to LSB_ERR_SHIFT and ADD_LSHIFT bits respectively.
3. Then, set the OD or GAMMA to dithering mode depends on path-1 or path-2.
Signed-off-by: Bibby Hsieh
---
drivers/gpu/drm/mediatek/mtk_disp_ovl.c |3 +-
drivers/gpu/drm/mediatek/mtk_disp_rdma.c|3 +-
drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 21 ++--
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 77 +++
drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h |6 +--
5 files changed, 92 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 8f62671f..019b7ca 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -103,7 +103,8 @@ static void mtk_ovl_stop(struct mtk_ddp_comp *comp)
}
static void mtk_ovl_config(struct mtk_ddp_comp *comp, unsigned int w,
- unsigned int h, unsigned int vrefresh)
+ unsigned int h, unsigned int vrefresh,
+ unsigned int bpc)
{
if (w != 0 && h != 0)
writel_relaxed(h << 16 | w, comp->regs + DISP_REG_OVL_ROI_SIZE);
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
index 5fb80cb..0df05f9 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
@@ -106,7 +106,8 @@ static void mtk_rdma_stop(struct mtk_ddp_comp *comp)
}
static void mtk_rdma_config(struct mtk_ddp_comp *comp, unsigned int width,
- unsigned int height, unsigned int vrefresh)
+ unsigned int height, unsigned int vrefresh,
+ unsigned int bpc)
{
unsigned int threshold;
unsigned int reg;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index ee219bb..18c9d0d 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -222,7 +222,9 @@ static void mtk_crtc_ddp_clk_disable(struct mtk_drm_crtc
*mtk_crtc)
static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc *mtk_crtc)
{
struct drm_crtc *crtc = _crtc->base;
- unsigned int width, height, vrefresh;
+ struct drm_connector *connector;
+ struct drm_encoder *encoder;
+ unsigned int width, height, vrefresh, bpc = 0;
int ret;
int i;
@@ -234,6 +236,19 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
*mtk_crtc)
height = crtc->state->adjusted_mode.vdisplay;
vrefresh = crtc->state->adjusted_mode.vrefresh;
+ drm_for_each_encoder(encoder, crtc->dev) {
+ if (encoder->crtc != crtc)
+ continue;
+
+ drm_for_each_connector(connector, crtc->dev) {
+ if (connector->encoder != encoder)
+ continue;
+ if (connector->display_info.bpc >= 3 &&
+ (bpc == 0 || bpc > connector->display_info.bpc))
+ bpc = connector->display_info.bpc;
+ }
+ }
+
ret = pm_runtime_get_sync(crtc->dev->dev);
if (ret < 0) {
DRM_ERROR("Failed to enable power domain: %d\n", ret);
@@ -266,7 +281,7 @@ static int mtk_crtc_ddp_hw_init(struct mtk_drm_crtc
*mtk_crtc)
for (i = 0; i < mtk_crtc->ddp_comp_nr; i++) {
struct mtk_ddp_comp *comp = mtk_crtc->ddp_comp[i];
- mtk_ddp_comp_config(comp, width, height, vrefresh);
+ mtk_ddp_comp_config(comp, width, height, vrefresh, bpc);
mtk_ddp_comp_start(comp);
}
@@ -469,7 +484,7 @@ void mtk_crtc_ddp_irq(struct drm_crtc *crtc, struct
mtk_ddp_comp *ovl)
if (state->pending_config) {
mtk_ddp_comp_config(ovl, state->pending_width,
state->pending_height,
- state->pending_vrefresh);
+ state->pending_vrefresh, 0);
state->pending_config = false;
}
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
index 56c5894..c10faac 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c
@@ -30,7 +30,26 @@
#define
