| Message ID | 20190314025838.5867-1-anarsoul@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC] drm/sun4i: Implement gamma correction | expand |
Hi Vasily, On Wed, Mar 13, 2019 at 07:58:38PM -0700, Vasily Khoruzhick wrote: > Add support for gamma corretion to sun4i TCON driver. Its LUT has 256 > entries and can be updated only when gamma correction is disabled. > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> It's not really clear to me what you expect a comment on? Maxime > --- > drivers/gpu/drm/sun4i/sun4i_crtc.c | 15 ++++++++++++++ > drivers/gpu/drm/sun4i/sun4i_tcon.c | 33 ++++++++++++++++++++++++++++++ > drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 ++++++++++- > 3 files changed, 59 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c > index 3eedf335a935..719259d09632 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c > +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c > @@ -101,6 +101,20 @@ static void sun4i_crtc_atomic_flush(struct drm_crtc *crtc, > drm_crtc_send_vblank_event(crtc, event); > spin_unlock_irq(&crtc->dev->event_lock); > } > + > + if (crtc->state->color_mgmt_changed) { > + if (crtc->state->gamma_lut) { > + /* LUT can be only updated when gamma correction is > + * disabled > + */ > + sun4i_tcon_enable_gamma(scrtc->tcon, false); > + sun4i_tcon_load_gamma_lut(scrtc->tcon, > + crtc->state->gamma_lut->data); > + sun4i_tcon_enable_gamma(scrtc->tcon, true); > + } else > + sun4i_tcon_enable_gamma(scrtc->tcon, false); > + } > + > } > > static void sun4i_crtc_atomic_disable(struct drm_crtc *crtc, > @@ -184,6 +198,7 @@ static const struct drm_crtc_funcs sun4i_crtc_funcs = { > .set_config = drm_atomic_helper_set_config, > .enable_vblank = sun4i_crtc_enable_vblank, > .disable_vblank = sun4i_crtc_disable_vblank, > + .gamma_set = drm_atomic_helper_legacy_gamma_set, > }; > > struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm, > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > index cf45d0f940f9..3f5f9d4f54a6 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > @@ -215,6 +215,34 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) > } > EXPORT_SYMBOL(sun4i_tcon_enable_vblank); > > +void sun4i_tcon_load_gamma_lut(struct sun4i_tcon *tcon, > + struct drm_color_lut *lut) > +{ > + int i; > + > + for (i = 0; i < SUN4I_TCON_GAMMA_LUT_SIZE; i++) { > + u32 r, g, b; > + > + r = drm_color_lut_extract(lut[i].red, 8); > + g = drm_color_lut_extract(lut[i].green, 8); > + b = drm_color_lut_extract(lut[i].blue, 8); > + > + regmap_write(tcon->regs, SUN4I_TCON_GAMMA_TABLE_REG + 4 * i, > + SUN4I_TCON_GAMMA_TABLE_R(r) | > + SUN4I_TCON_GAMMA_TABLE_G(g) | > + SUN4I_TCON_GAMMA_TABLE_B(b)); > + } > +} > +EXPORT_SYMBOL(sun4i_tcon_load_gamma_lut); > + > +void sun4i_tcon_enable_gamma(struct sun4i_tcon *tcon, bool enable) > +{ > + regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG, > + SUN4I_TCON_GCTL_GAMMA_ENABLE, > + enable ? SUN4I_TCON_GCTL_GAMMA_ENABLE : 0); > +} > +EXPORT_SYMBOL(sun4i_tcon_enable_gamma); > + > /* > * This function is a helper for TCON output muxing. The TCON output > * muxing control register in earlier SoCs (without the TCON TOP block) > @@ -1261,6 +1289,11 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, > > list_add_tail(&tcon->list, &drv->tcon_list); > > + drm_mode_crtc_set_gamma_size(&tcon->crtc->crtc, > + SUN4I_TCON_GAMMA_LUT_SIZE); > + drm_crtc_enable_color_mgmt(&tcon->crtc->crtc, 0, false, > + tcon->crtc->crtc.gamma_size); > + > return 0; > > err_free_dotclock: > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > index 84cfb1952ff7..68a29e49e426 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > @@ -22,6 +22,7 @@ > > #define SUN4I_TCON_GCTL_REG 0x0 > #define SUN4I_TCON_GCTL_TCON_ENABLE BIT(31) > +#define SUN4I_TCON_GCTL_GAMMA_ENABLE BIT(30) > #define SUN4I_TCON_GCTL_IOMAP_MASK BIT(0) > #define SUN4I_TCON_GCTL_IOMAP_TCON1 (1 << 0) > #define SUN4I_TCON_GCTL_IOMAP_TCON0 (0 << 0) > @@ -215,7 +216,13 @@ > #define SUN4I_TCON1_FILL_BEG2_REG 0x31c > #define SUN4I_TCON1_FILL_END2_REG 0x320 > #define SUN4I_TCON1_FILL_DATA2_REG 0x324 > -#define SUN4I_TCON1_GAMMA_TABLE_REG 0x400 > + > +#define SUN4I_TCON_GAMMA_TABLE_REG 0x400 > +#define SUN4I_TCON_GAMMA_TABLE_B(x) ((x) & 0xff) > +#define SUN4I_TCON_GAMMA_TABLE_G(x) (((x) & 0xff) << 8) > +#define SUN4I_TCON_GAMMA_TABLE_R(x) (((x) & 0xff) << 16) > + > +#define SUN4I_TCON_GAMMA_LUT_SIZE 256 > > #define SUN4I_TCON_MAX_CHANNELS 2 > > @@ -278,6 +285,9 @@ void sun4i_tcon_mode_set(struct sun4i_tcon *tcon, > const struct drm_display_mode *mode); > void sun4i_tcon_set_status(struct sun4i_tcon *crtc, > const struct drm_encoder *encoder, bool enable); > +void sun4i_tcon_load_gamma_lut(struct sun4i_tcon *tcon, > + struct drm_color_lut *lut); > +void sun4i_tcon_enable_gamma(struct sun4i_tcon *tcon, bool enable); > > extern const struct of_device_id sun4i_tcon_of_table[]; > > -- > 2.21.0 >
On Thu, Mar 14, 2019 at 8:42 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > Hi Vasily, > > On Wed, Mar 13, 2019 at 07:58:38PM -0700, Vasily Khoruzhick wrote: > > Add support for gamma corretion to sun4i TCON driver. Its LUT has 256 > > entries and can be updated only when gamma correction is disabled. > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > It's not really clear to me what you expect a comment on? I'm not sure that I put gamma correction into right place - gamma lut seems to be a property of CRTC, but registers are in TCON module. Also I'm not sure if gamma correction is supported across all Allwinner SoCs - if it doesn't, how do we handle this? > > Maxime > > > --- > > drivers/gpu/drm/sun4i/sun4i_crtc.c | 15 ++++++++++++++ > > drivers/gpu/drm/sun4i/sun4i_tcon.c | 33 ++++++++++++++++++++++++++++++ > > drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 ++++++++++- > > 3 files changed, 59 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c > > index 3eedf335a935..719259d09632 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c > > @@ -101,6 +101,20 @@ static void sun4i_crtc_atomic_flush(struct drm_crtc *crtc, > > drm_crtc_send_vblank_event(crtc, event); > > spin_unlock_irq(&crtc->dev->event_lock); > > } > > + > > + if (crtc->state->color_mgmt_changed) { > > + if (crtc->state->gamma_lut) { > > + /* LUT can be only updated when gamma correction is > > + * disabled > > + */ > > + sun4i_tcon_enable_gamma(scrtc->tcon, false); > > + sun4i_tcon_load_gamma_lut(scrtc->tcon, > > + crtc->state->gamma_lut->data); > > + sun4i_tcon_enable_gamma(scrtc->tcon, true); > > + } else > > + sun4i_tcon_enable_gamma(scrtc->tcon, false); > > + } > > + > > } > > > > static void sun4i_crtc_atomic_disable(struct drm_crtc *crtc, > > @@ -184,6 +198,7 @@ static const struct drm_crtc_funcs sun4i_crtc_funcs = { > > .set_config = drm_atomic_helper_set_config, > > .enable_vblank = sun4i_crtc_enable_vblank, > > .disable_vblank = sun4i_crtc_disable_vblank, > > + .gamma_set = drm_atomic_helper_legacy_gamma_set, > > }; > > > > struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm, > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > index cf45d0f940f9..3f5f9d4f54a6 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c > > @@ -215,6 +215,34 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) > > } > > EXPORT_SYMBOL(sun4i_tcon_enable_vblank); > > > > +void sun4i_tcon_load_gamma_lut(struct sun4i_tcon *tcon, > > + struct drm_color_lut *lut) > > +{ > > + int i; > > + > > + for (i = 0; i < SUN4I_TCON_GAMMA_LUT_SIZE; i++) { > > + u32 r, g, b; > > + > > + r = drm_color_lut_extract(lut[i].red, 8); > > + g = drm_color_lut_extract(lut[i].green, 8); > > + b = drm_color_lut_extract(lut[i].blue, 8); > > + > > + regmap_write(tcon->regs, SUN4I_TCON_GAMMA_TABLE_REG + 4 * i, > > + SUN4I_TCON_GAMMA_TABLE_R(r) | > > + SUN4I_TCON_GAMMA_TABLE_G(g) | > > + SUN4I_TCON_GAMMA_TABLE_B(b)); > > + } > > +} > > +EXPORT_SYMBOL(sun4i_tcon_load_gamma_lut); > > + > > +void sun4i_tcon_enable_gamma(struct sun4i_tcon *tcon, bool enable) > > +{ > > + regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG, > > + SUN4I_TCON_GCTL_GAMMA_ENABLE, > > + enable ? SUN4I_TCON_GCTL_GAMMA_ENABLE : 0); > > +} > > +EXPORT_SYMBOL(sun4i_tcon_enable_gamma); > > + > > /* > > * This function is a helper for TCON output muxing. The TCON output > > * muxing control register in earlier SoCs (without the TCON TOP block) > > @@ -1261,6 +1289,11 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, > > > > list_add_tail(&tcon->list, &drv->tcon_list); > > > > + drm_mode_crtc_set_gamma_size(&tcon->crtc->crtc, > > + SUN4I_TCON_GAMMA_LUT_SIZE); > > + drm_crtc_enable_color_mgmt(&tcon->crtc->crtc, 0, false, > > + tcon->crtc->crtc.gamma_size); > > + > > return 0; > > > > err_free_dotclock: > > diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h > > index 84cfb1952ff7..68a29e49e426 100644 > > --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h > > +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h > > @@ -22,6 +22,7 @@ > > > > #define SUN4I_TCON_GCTL_REG 0x0 > > #define SUN4I_TCON_GCTL_TCON_ENABLE BIT(31) > > +#define SUN4I_TCON_GCTL_GAMMA_ENABLE BIT(30) > > #define SUN4I_TCON_GCTL_IOMAP_MASK BIT(0) > > #define SUN4I_TCON_GCTL_IOMAP_TCON1 (1 << 0) > > #define SUN4I_TCON_GCTL_IOMAP_TCON0 (0 << 0) > > @@ -215,7 +216,13 @@ > > #define SUN4I_TCON1_FILL_BEG2_REG 0x31c > > #define SUN4I_TCON1_FILL_END2_REG 0x320 > > #define SUN4I_TCON1_FILL_DATA2_REG 0x324 > > -#define SUN4I_TCON1_GAMMA_TABLE_REG 0x400 > > + > > +#define SUN4I_TCON_GAMMA_TABLE_REG 0x400 > > +#define SUN4I_TCON_GAMMA_TABLE_B(x) ((x) & 0xff) > > +#define SUN4I_TCON_GAMMA_TABLE_G(x) (((x) & 0xff) << 8) > > +#define SUN4I_TCON_GAMMA_TABLE_R(x) (((x) & 0xff) << 16) > > + > > +#define SUN4I_TCON_GAMMA_LUT_SIZE 256 > > > > #define SUN4I_TCON_MAX_CHANNELS 2 > > > > @@ -278,6 +285,9 @@ void sun4i_tcon_mode_set(struct sun4i_tcon *tcon, > > const struct drm_display_mode *mode); > > void sun4i_tcon_set_status(struct sun4i_tcon *crtc, > > const struct drm_encoder *encoder, bool enable); > > +void sun4i_tcon_load_gamma_lut(struct sun4i_tcon *tcon, > > + struct drm_color_lut *lut); > > +void sun4i_tcon_enable_gamma(struct sun4i_tcon *tcon, bool enable); > > > > extern const struct of_device_id sun4i_tcon_of_table[]; > > > > -- > > 2.21.0 > > > > -- > Maxime Ripard, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com > -----BEGIN PGP SIGNATURE----- > > iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCXIp2SwAKCRDj7w1vZxhR > xSnbAP9ZiYBt1fgCp+KW1N+OWLfacqrrgWPEWsI4Mqlhstt55AD9FXYVo74SWFUO > pq01CNG+Ci/cAN7u77wT3jaxgL7qvQc= > =TuHd > -----END PGP SIGNATURE-----
Hi, On Thu, Mar 14, 2019 at 10:54:26AM -0700, Vasily Khoruzhick wrote: > On Thu, Mar 14, 2019 at 8:42 AM Maxime Ripard <maxime.ripard@bootlin.com> wrote: > > > > Hi Vasily, > > > > On Wed, Mar 13, 2019 at 07:58:38PM -0700, Vasily Khoruzhick wrote: > > > Add support for gamma corretion to sun4i TCON driver. Its LUT has 256 > > > entries and can be updated only when gamma correction is disabled. > > > > > > Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> > > > > It's not really clear to me what you expect a comment on? > > I'm not sure that I put gamma correction into right place - gamma lut > seems to be a property of CRTC, but registers are in TCON module. The TCON loosely maps to the CRTC, so that's ok. > Also I'm not sure if gamma correction is supported across all > Allwinner SoCs - if it doesn't, how do we handle this? I don't really know if that's supported on all of them, but the way to handle this would be to associate it with the compatible and add a quirk if that's relevant. Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c index 3eedf335a935..719259d09632 100644 --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c @@ -101,6 +101,20 @@ static void sun4i_crtc_atomic_flush(struct drm_crtc *crtc, drm_crtc_send_vblank_event(crtc, event); spin_unlock_irq(&crtc->dev->event_lock); } + + if (crtc->state->color_mgmt_changed) { + if (crtc->state->gamma_lut) { + /* LUT can be only updated when gamma correction is + * disabled + */ + sun4i_tcon_enable_gamma(scrtc->tcon, false); + sun4i_tcon_load_gamma_lut(scrtc->tcon, + crtc->state->gamma_lut->data); + sun4i_tcon_enable_gamma(scrtc->tcon, true); + } else + sun4i_tcon_enable_gamma(scrtc->tcon, false); + } + } static void sun4i_crtc_atomic_disable(struct drm_crtc *crtc, @@ -184,6 +198,7 @@ static const struct drm_crtc_funcs sun4i_crtc_funcs = { .set_config = drm_atomic_helper_set_config, .enable_vblank = sun4i_crtc_enable_vblank, .disable_vblank = sun4i_crtc_disable_vblank, + .gamma_set = drm_atomic_helper_legacy_gamma_set, }; struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm, diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index cf45d0f940f9..3f5f9d4f54a6 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -215,6 +215,34 @@ void sun4i_tcon_enable_vblank(struct sun4i_tcon *tcon, bool enable) } EXPORT_SYMBOL(sun4i_tcon_enable_vblank); +void sun4i_tcon_load_gamma_lut(struct sun4i_tcon *tcon, + struct drm_color_lut *lut) +{ + int i; + + for (i = 0; i < SUN4I_TCON_GAMMA_LUT_SIZE; i++) { + u32 r, g, b; + + r = drm_color_lut_extract(lut[i].red, 8); + g = drm_color_lut_extract(lut[i].green, 8); + b = drm_color_lut_extract(lut[i].blue, 8); + + regmap_write(tcon->regs, SUN4I_TCON_GAMMA_TABLE_REG + 4 * i, + SUN4I_TCON_GAMMA_TABLE_R(r) | + SUN4I_TCON_GAMMA_TABLE_G(g) | + SUN4I_TCON_GAMMA_TABLE_B(b)); + } +} +EXPORT_SYMBOL(sun4i_tcon_load_gamma_lut); + +void sun4i_tcon_enable_gamma(struct sun4i_tcon *tcon, bool enable) +{ + regmap_update_bits(tcon->regs, SUN4I_TCON_GCTL_REG, + SUN4I_TCON_GCTL_GAMMA_ENABLE, + enable ? SUN4I_TCON_GCTL_GAMMA_ENABLE : 0); +} +EXPORT_SYMBOL(sun4i_tcon_enable_gamma); + /* * This function is a helper for TCON output muxing. The TCON output * muxing control register in earlier SoCs (without the TCON TOP block) @@ -1261,6 +1289,11 @@ static int sun4i_tcon_bind(struct device *dev, struct device *master, list_add_tail(&tcon->list, &drv->tcon_list); + drm_mode_crtc_set_gamma_size(&tcon->crtc->crtc, + SUN4I_TCON_GAMMA_LUT_SIZE); + drm_crtc_enable_color_mgmt(&tcon->crtc->crtc, 0, false, + tcon->crtc->crtc.gamma_size); + return 0; err_free_dotclock: diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.h b/drivers/gpu/drm/sun4i/sun4i_tcon.h index 84cfb1952ff7..68a29e49e426 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.h +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.h @@ -22,6 +22,7 @@ #define SUN4I_TCON_GCTL_REG 0x0 #define SUN4I_TCON_GCTL_TCON_ENABLE BIT(31) +#define SUN4I_TCON_GCTL_GAMMA_ENABLE BIT(30) #define SUN4I_TCON_GCTL_IOMAP_MASK BIT(0) #define SUN4I_TCON_GCTL_IOMAP_TCON1 (1 << 0) #define SUN4I_TCON_GCTL_IOMAP_TCON0 (0 << 0) @@ -215,7 +216,13 @@ #define SUN4I_TCON1_FILL_BEG2_REG 0x31c #define SUN4I_TCON1_FILL_END2_REG 0x320 #define SUN4I_TCON1_FILL_DATA2_REG 0x324 -#define SUN4I_TCON1_GAMMA_TABLE_REG 0x400 + +#define SUN4I_TCON_GAMMA_TABLE_REG 0x400 +#define SUN4I_TCON_GAMMA_TABLE_B(x) ((x) & 0xff) +#define SUN4I_TCON_GAMMA_TABLE_G(x) (((x) & 0xff) << 8) +#define SUN4I_TCON_GAMMA_TABLE_R(x) (((x) & 0xff) << 16) + +#define SUN4I_TCON_GAMMA_LUT_SIZE 256 #define SUN4I_TCON_MAX_CHANNELS 2 @@ -278,6 +285,9 @@ void sun4i_tcon_mode_set(struct sun4i_tcon *tcon, const struct drm_display_mode *mode); void sun4i_tcon_set_status(struct sun4i_tcon *crtc, const struct drm_encoder *encoder, bool enable); +void sun4i_tcon_load_gamma_lut(struct sun4i_tcon *tcon, + struct drm_color_lut *lut); +void sun4i_tcon_enable_gamma(struct sun4i_tcon *tcon, bool enable); extern const struct of_device_id sun4i_tcon_of_table[];
Add support for gamma corretion to sun4i TCON driver. Its LUT has 256 entries and can be updated only when gamma correction is disabled. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- drivers/gpu/drm/sun4i/sun4i_crtc.c | 15 ++++++++++++++ drivers/gpu/drm/sun4i/sun4i_tcon.c | 33 ++++++++++++++++++++++++++++++ drivers/gpu/drm/sun4i/sun4i_tcon.h | 12 ++++++++++- 3 files changed, 59 insertions(+), 1 deletion(-)