On Wed, Aug 22, 2018 at 04:18:19PM +0100, Alexandru Gheorghe wrote: > When we want to writeback to memory in NV12 format we need to program > the RGB2YUV coefficients. Currently, we don't program the coefficients > and NV12 doesn't work at all. > > This patchset fixes that by programming a sane default(bt709, limited > range) as rgb2yuv coefficients. > > In the long run, probably we need to think of a way for userspace to > be able to program that, but for now I think this is better than not > working at all or not advertising NV12 as a supported format for > memwrite. > > Changes since v1: > - Write the rgb2yuv coefficients only once, since we don't change > them at all, just write them the first time NV12 is programmed, > suggested by Brian Starkey, here [1] > > [1] https://lists.freedesktop.org/archives/dri-devel/2018-August/186819.html > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx> Acked-by: Liviu Dudau <liviu.dudau@xxxxxxx> > --- > drivers/gpu/drm/arm/malidp_hw.c | 25 +++++++++++++++++++++++-- > drivers/gpu/drm/arm/malidp_hw.h | 3 ++- > drivers/gpu/drm/arm/malidp_mw.c | 25 +++++++++++++++++++++---- > drivers/gpu/drm/arm/malidp_regs.h | 2 ++ > 4 files changed, 48 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c > index c94a4422e0e9..2781e462c1ed 100644 > --- a/drivers/gpu/drm/arm/malidp_hw.c > +++ b/drivers/gpu/drm/arm/malidp_hw.c > @@ -384,7 +384,8 @@ static long malidp500_se_calc_mclk(struct malidp_hw_device *hwdev, > > static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev, > dma_addr_t *addrs, s32 *pitches, > - int num_planes, u16 w, u16 h, u32 fmt_id) > + int num_planes, u16 w, u16 h, u32 fmt_id, > + const s16 *rgb2yuv_coeffs) > { > u32 base = MALIDP500_SE_MEMWRITE_BASE; > u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK); > @@ -416,6 +417,16 @@ static int malidp500_enable_memwrite(struct malidp_hw_device *hwdev, > > malidp_hw_write(hwdev, MALIDP_DE_H_ACTIVE(w) | MALIDP_DE_V_ACTIVE(h), > MALIDP500_SE_MEMWRITE_OUT_SIZE); > + > + if (rgb2yuv_coeffs) { > + int i; > + > + for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) { > + malidp_hw_write(hwdev, rgb2yuv_coeffs[i], > + MALIDP500_SE_RGB_YUV_COEFFS + i * 4); > + } > + } > + > malidp_hw_setbits(hwdev, MALIDP_SE_MEMWRITE_EN, MALIDP500_SE_CONTROL); > > return 0; > @@ -658,7 +669,8 @@ static long malidp550_se_calc_mclk(struct malidp_hw_device *hwdev, > > static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev, > dma_addr_t *addrs, s32 *pitches, > - int num_planes, u16 w, u16 h, u32 fmt_id) > + int num_planes, u16 w, u16 h, u32 fmt_id, > + const s16 *rgb2yuv_coeffs) > { > u32 base = MALIDP550_SE_MEMWRITE_BASE; > u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK); > @@ -689,6 +701,15 @@ static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev, > malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN, > MALIDP550_SE_CONTROL); > > + if (rgb2yuv_coeffs) { > + int i; > + > + for (i = 0; i < MALIDP_COLORADJ_NUM_COEFFS; i++) { > + malidp_hw_write(hwdev, rgb2yuv_coeffs[i], > + MALIDP550_SE_RGB_YUV_COEFFS + i * 4); > + } > + } > + > return 0; > } > > diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h > index ad2e96915d44..9fc94c08190f 100644 > --- a/drivers/gpu/drm/arm/malidp_hw.h > +++ b/drivers/gpu/drm/arm/malidp_hw.h > @@ -191,7 +191,8 @@ struct malidp_hw { > * @param fmt_id - internal format ID of output buffer > */ > int (*enable_memwrite)(struct malidp_hw_device *hwdev, dma_addr_t *addrs, > - s32 *pitches, int num_planes, u16 w, u16 h, u32 fmt_id); > + s32 *pitches, int num_planes, u16 w, u16 h, u32 fmt_id, > + const s16 *rgb2yuv_coeffs); > > /* > * Disable the writing to memory of the next frame's content. > diff --git a/drivers/gpu/drm/arm/malidp_mw.c b/drivers/gpu/drm/arm/malidp_mw.c > index cfd718e7e97c..429a11e6473b 100644 > --- a/drivers/gpu/drm/arm/malidp_mw.c > +++ b/drivers/gpu/drm/arm/malidp_mw.c > @@ -26,6 +26,8 @@ struct malidp_mw_connector_state { > s32 pitches[2]; > u8 format; > u8 n_planes; > + bool rgb2yuv_initialized; > + const s16 *rgb2yuv_coeffs; > }; > > static int malidp_mw_connector_get_modes(struct drm_connector *connector) > @@ -84,7 +86,7 @@ static void malidp_mw_connector_destroy(struct drm_connector *connector) > static struct drm_connector_state * > malidp_mw_connector_duplicate_state(struct drm_connector *connector) > { > - struct malidp_mw_connector_state *mw_state; > + struct malidp_mw_connector_state *mw_state, *mw_current_state; > > if (WARN_ON(!connector->state)) > return NULL; > @@ -93,7 +95,10 @@ malidp_mw_connector_duplicate_state(struct drm_connector *connector) > if (!mw_state) > return NULL; > > - /* No need to preserve any of our driver-local data */ > + mw_current_state = to_mw_state(connector->state); > + mw_state->rgb2yuv_coeffs = mw_current_state->rgb2yuv_coeffs; > + mw_state->rgb2yuv_initialized = mw_current_state->rgb2yuv_initialized; > + > __drm_atomic_helper_connector_duplicate_state(connector, &mw_state->base); > > return &mw_state->base; > @@ -108,6 +113,13 @@ static const struct drm_connector_funcs malidp_mw_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > +static const s16 rgb2yuv_coeffs_bt709_limited[MALIDP_COLORADJ_NUM_COEFFS] = { > + 47, 157, 16, > + -26, -87, 112, > + 112, -102, -10, > + 16, 128, 128 > +}; > + > static int > malidp_mw_encoder_atomic_check(struct drm_encoder *encoder, > struct drm_crtc_state *crtc_state, > @@ -157,6 +169,9 @@ malidp_mw_encoder_atomic_check(struct drm_encoder *encoder, > } > mw_state->n_planes = n_planes; > > + if (fb->format->is_yuv) > + mw_state->rgb2yuv_coeffs = rgb2yuv_coeffs_bt709_limited; > + > return 0; > } > > @@ -239,10 +254,12 @@ void malidp_mw_atomic_commit(struct drm_device *drm, > > drm_writeback_queue_job(mw_conn, conn_state->writeback_job); > conn_state->writeback_job = NULL; > - > hwdev->hw->enable_memwrite(hwdev, mw_state->addrs, > mw_state->pitches, mw_state->n_planes, > - fb->width, fb->height, mw_state->format); > + fb->width, fb->height, mw_state->format, > + !mw_state->rgb2yuv_initialized ? > + mw_state->rgb2yuv_coeffs : NULL); > + mw_state->rgb2yuv_initialized = !!mw_state->rgb2yuv_coeffs; > } else { > DRM_DEV_DEBUG_DRIVER(drm->dev, "Disable memwrite\n"); > hwdev->hw->disable_memwrite(hwdev); > diff --git a/drivers/gpu/drm/arm/malidp_regs.h b/drivers/gpu/drm/arm/malidp_regs.h > index 3579d36b2a71..6ffe849774f2 100644 > --- a/drivers/gpu/drm/arm/malidp_regs.h > +++ b/drivers/gpu/drm/arm/malidp_regs.h > @@ -205,6 +205,7 @@ > #define MALIDP500_SE_BASE 0x00c00 > #define MALIDP500_SE_CONTROL 0x00c0c > #define MALIDP500_SE_MEMWRITE_OUT_SIZE 0x00c2c > +#define MALIDP500_SE_RGB_YUV_COEFFS 0x00C74 > #define MALIDP500_SE_MEMWRITE_BASE 0x00e00 > #define MALIDP500_DC_IRQ_BASE 0x00f00 > #define MALIDP500_CONFIG_VALID 0x00f00 > @@ -238,6 +239,7 @@ > #define MALIDP550_SE_CONTROL 0x08010 > #define MALIDP550_SE_MEMWRITE_ONESHOT (1 << 7) > #define MALIDP550_SE_MEMWRITE_OUT_SIZE 0x08030 > +#define MALIDP550_SE_RGB_YUV_COEFFS 0x08078 > #define MALIDP550_SE_MEMWRITE_BASE 0x08100 > #define MALIDP550_DC_BASE 0x0c000 > #define MALIDP550_DC_CONTROL 0x0c010 > -- > 2.18.0 > -- ==================== | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --------------- ¯\_(ツ)_/¯ _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel