| Message ID | 1497604347-17960-2-git-send-email-peda@axentia.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2017-06-16 12:01, Boris Brezillon wrote: > Hi Peter, > > On Fri, 16 Jun 2017 11:12:25 +0200 > Peter Rosin <peda@axentia.se> wrote: > >> All layers of chips support this, the only variable is the base address >> of the lookup table in the register map. >> >> Signed-off-by: Peter Rosin <peda@axentia.se> >> --- >> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 48 +++++++++++++++++++++++++ >> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 +++++++ >> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 16 +++++++++ >> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +++ >> 4 files changed, 82 insertions(+) >> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c >> index 5348985..75871b5 100644 >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c >> @@ -61,6 +61,7 @@ struct atmel_hlcdc_crtc { >> struct atmel_hlcdc_dc *dc; >> struct drm_pending_vblank_event *event; >> int id; >> + u32 clut[ATMEL_HLCDC_CLUT_SIZE]; > > Do we really need to duplicate this table here? I mean, the gamma_lut > table should always be available in the crtc_state, so do you have a > good reason a copy here? If I don't keep a copy in the driver, it doesn't work when there's no gamma_lut. And there is no gamma_lut when I use fbdev emulation. Maybe that's a bug somewhere else? Sure, I could have added it in patch 3/3 instead, but didn't when I divided the work into patches... >> }; >> >> static inline struct atmel_hlcdc_crtc * >> @@ -140,6 +141,46 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) >> cfg); >> } >> >> +static void >> +atmel_hlcdc_crtc_load_lut(struct drm_crtc *c) >> +{ >> + struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c); >> + struct atmel_hlcdc_dc *dc = crtc->dc; >> + int layer; >> + int idx; >> + >> + for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) { >> + if (!dc->layers[layer]) >> + continue; >> + for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) >> + atmel_hlcdc_layer_write_clut(dc->layers[layer], >> + idx, crtc->clut[idx]); >> + } >> +} >> + >> +static void >> +atmel_hlcdc_crtc_flush_lut(struct drm_crtc *c) >> +{ >> + struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c); >> + struct drm_crtc_state *state = c->state; >> + struct drm_color_lut *lut; >> + int idx; >> + >> + if (!state->gamma_lut) >> + return; >> + >> + lut = (struct drm_color_lut *)state->gamma_lut->data; >> + >> + for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) { >> + crtc->clut[idx] = >> + ((lut[idx].red << 8) & 0xff0000) | >> + (lut[idx].green & 0xff00) | >> + (lut[idx].blue >> 8); >> + } >> + >> + atmel_hlcdc_crtc_load_lut(c); >> +} >> + >> static enum drm_mode_status >> atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c, >> const struct drm_display_mode *mode) >> @@ -312,6 +353,9 @@ static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc, >> struct drm_crtc_state *old_s) >> { >> /* TODO: write common plane control register if available */ >> + >> + if (crtc->state->color_mgmt_changed) >> + atmel_hlcdc_crtc_flush_lut(crtc); > > Hm, it's probably too late to do it here. Planes have already been > enabled and the engine may have started to fetch data and do the > composition. You could do that in ->update_plane() [1], and make it a > per-plane thing. > > I'm not sure, but I think you can get the new crtc_state from > plane->crtc->state in this context (state have already been swapped, > and new state is being applied, which means relevant locks are held). Ok, I can move it there. My plan is to just copy the default .update_plane function and insert if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut) { ... } just before the drm_atomic_commit(state) call. Sounds ok? >> } >> >> static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = { >> @@ -429,6 +473,7 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = { >> .atomic_destroy_state = atmel_hlcdc_crtc_destroy_state, >> .enable_vblank = atmel_hlcdc_crtc_enable_vblank, >> .disable_vblank = atmel_hlcdc_crtc_disable_vblank, >> + .set_property = drm_atomic_helper_crtc_set_property, > > Well, this change is independent from gamma LUT support. Should > probably be done in a separate patch. Ok, I think I fat-fingered some kernel cmdline at some point and fooled myself into thinking I needed it for some reason... > Also, you should probably have: > > .gamma_set = drm_atomic_helper_legacy_gamma_set, That doesn't no anything for me, but sure, I can add it. Cheers, peda >> }; > > The rest looks good. > > Thanks, > > Boris >
On 2017-06-16 18:15, Boris Brezillon wrote: > Hi Peter, > > On Fri, 16 Jun 2017 17:54:04 +0200 > Peter Rosin <peda@axentia.se> wrote: > >> On 2017-06-16 12:01, Boris Brezillon wrote: >>> Hi Peter, >>> >>> On Fri, 16 Jun 2017 11:12:25 +0200 >>> Peter Rosin <peda@axentia.se> wrote: >>> >>>> All layers of chips support this, the only variable is the base address >>>> of the lookup table in the register map. >>>> >>>> Signed-off-by: Peter Rosin <peda@axentia.se> >>>> --- >>>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 48 +++++++++++++++++++++++++ >>>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 +++++++ >>>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 16 +++++++++ >>>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +++ >>>> 4 files changed, 82 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c >>>> index 5348985..75871b5 100644 >>>> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c >>>> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c >>>> @@ -61,6 +61,7 @@ struct atmel_hlcdc_crtc { >>>> struct atmel_hlcdc_dc *dc; >>>> struct drm_pending_vblank_event *event; >>>> int id; >>>> + u32 clut[ATMEL_HLCDC_CLUT_SIZE]; >>> >>> Do we really need to duplicate this table here? I mean, the gamma_lut >>> table should always be available in the crtc_state, so do you have a >>> good reason a copy here? >> >> If I don't keep a copy in the driver, it doesn't work when there's no >> gamma_lut. And there is no gamma_lut when I use fbdev emulation. Maybe >> that's a bug somewhere else? > > Can't we re-use crtc->gamma_store? Honnestly, I don't know how the > fbdev->DRM link should be done, so we'd better wait for DRM maintainers > feedback here (Daniel, any opinion?). Ahh, gamma_store. Makes perfect sense. Thanks for that pointer! >> >> Sure, I could have added it in patch 3/3 instead, but didn't when I >> divided the work into patches... > > No, my point is that IMO it shouldn't be needed at all. Right, with gamma_store it's no longer needed. >> >>>> }; >>>> >>>> static inline struct atmel_hlcdc_crtc * >>>> @@ -140,6 +141,46 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) >>>> cfg); >>>> } >>>> >>>> +static void >>>> +atmel_hlcdc_crtc_load_lut(struct drm_crtc *c) >>>> +{ >>>> + struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c); >>>> + struct atmel_hlcdc_dc *dc = crtc->dc; >>>> + int layer; >>>> + int idx; >>>> + >>>> + for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) { >>>> + if (!dc->layers[layer]) >>>> + continue; >>>> + for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) >>>> + atmel_hlcdc_layer_write_clut(dc->layers[layer], >>>> + idx, crtc->clut[idx]); >>>> + } >>>> +} >>>> + >>>> +static void >>>> +atmel_hlcdc_crtc_flush_lut(struct drm_crtc *c) >>>> +{ >>>> + struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c); >>>> + struct drm_crtc_state *state = c->state; >>>> + struct drm_color_lut *lut; >>>> + int idx; >>>> + >>>> + if (!state->gamma_lut) >>>> + return; >>>> + >>>> + lut = (struct drm_color_lut *)state->gamma_lut->data; >>>> + >>>> + for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) { >>>> + crtc->clut[idx] = >>>> + ((lut[idx].red << 8) & 0xff0000) | >>>> + (lut[idx].green & 0xff00) | >>>> + (lut[idx].blue >> 8); >>>> + } >>>> + >>>> + atmel_hlcdc_crtc_load_lut(c); >>>> +} >>>> + >>>> static enum drm_mode_status >>>> atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c, >>>> const struct drm_display_mode *mode) >>>> @@ -312,6 +353,9 @@ static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc, >>>> struct drm_crtc_state *old_s) >>>> { >>>> /* TODO: write common plane control register if available */ >>>> + >>>> + if (crtc->state->color_mgmt_changed) >>>> + atmel_hlcdc_crtc_flush_lut(crtc); >>> >>> Hm, it's probably too late to do it here. Planes have already been >>> enabled and the engine may have started to fetch data and do the >>> composition. You could do that in ->update_plane() [1], and make it a >>> per-plane thing. >>> >>> I'm not sure, but I think you can get the new crtc_state from >>> plane->crtc->state in this context (state have already been swapped, >>> and new state is being applied, which means relevant locks are held). >> >> Ok, I can move it there. My plan is to just copy the default .update_plane >> function and insert >> >> if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut) { >> ... >> } >> >> just before the drm_atomic_commit(state) call. Sounds ok? > > Why would you copy the default ->update_plane() when we already have > our own ->atomic_update_plane() implementation [1]? Just put it there > (before the atmel_hlcdc_layer_update_commit() call) and we should be > good. Ahh, but you said ->update_plane() and I took that as .update_plane in layer_plane_funcs, not ->atomic_update() in atmel_hlcdc_layer_plane_helper_funcs. Makes sense now, and much neater too. >> >>>> } >>>> >>>> static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = { >>>> @@ -429,6 +473,7 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = { >>>> .atomic_destroy_state = atmel_hlcdc_crtc_destroy_state, >>>> .enable_vblank = atmel_hlcdc_crtc_enable_vblank, >>>> .disable_vblank = atmel_hlcdc_crtc_disable_vblank, >>>> + .set_property = drm_atomic_helper_crtc_set_property, >>> >>> Well, this change is independent from gamma LUT support. Should >>> probably be done in a separate patch. >> >> Ok, I think I fat-fingered some kernel cmdline at some point and fooled >> myself into thinking I needed it for some reason... > > It's probably a good thing to have it set anyway. Looking at the code, I think it's needed since I think that's how the gamma_lut property is modified for the non-emulated case... >> >>> Also, you should probably have: >>> >>> .gamma_set = drm_atomic_helper_legacy_gamma_set, >> >> That doesn't no anything for me, but sure, I can add it. > > To be very clear, I'd like you to test it through DRM ioctls, not only > through the fbdev emulation layer. ...so yeah, right, I couldn't agree more. Any pointers to code w/o a bunch of complex library dependencies that I can test with? Cheers, peda
On 2017-06-16 23:12, Peter Rosin wrote: > On 2017-06-16 18:15, Boris Brezillon wrote: >> To be very clear, I'd like you to test it through DRM ioctls, not only >> through the fbdev emulation layer. > > ...so yeah, right, I couldn't agree more. Any pointers to code w/o a bunch > of complex library dependencies that I can test with? I have now built libdrm-2.4.81, and get this: $ modetest -M atmel-hlcdc -s 27@39:1024x768 setting mode 1024x768-60Hz@XR24 on connectors 27, crtc 39 $ modetest -M atmel-hlcdc -s 27@39:1024x768@RG16 setting mode 1024x768-60Hz@RG16 on connectors 27, crtc 39 $ modetest -M atmel-hlcdc -s 27@39:1024x768@C8 unknown format C8 <usage> (output on the lcd looks sane for the first two, not that I really know exactly what to expect) Looking at the libdrm code, I only find YUV and RGB modes in tests/util/format.c which make me less confident that I will find something sane to test with. So, pointers to code to test with desperately needed... Cheers, peda
>>>> Hm, it's probably too late to do it here. Planes have already been >>>> enabled and the engine may have started to fetch data and do the >>>> composition. You could do that in ->update_plane() [1], and make it a >>>> per-plane thing. >>>> >>>> I'm not sure, but I think you can get the new crtc_state from >>>> plane->crtc->state in this context (state have already been swapped, >>>> and new state is being applied, which means relevant locks are held). >>> >>> Ok, I can move it there. My plan is to just copy the default .update_plane >>> function and insert >>> >>> if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut) { >>> ... >>> } >>> >>> just before the drm_atomic_commit(state) call. Sounds ok? >> >> Why would you copy the default ->update_plane() when we already have >> our own ->atomic_update_plane() implementation [1]? Just put it there >> (before the atmel_hlcdc_layer_update_commit() call) and we should be >> good. > > Ahh, but you said ->update_plane() and I took that as .update_plane in > layer_plane_funcs, not ->atomic_update() in atmel_hlcdc_layer_plane_helper_funcs. > > Makes sense now, and much neater too. No, it doesn't make sense. There's no atmel_hlcdc_layer_update_commit call anywhere, and no such function. You seem to have some further changes that are not even in -next. Where am I getting those changes and why are they not upstream yet? There's a mention of the missing function here [1], but that's some 18 months ago. What's going on? [1] https://patchwork.kernel.org/patch/7965721/ Cheers, peda
On 2017-06-17 07:36, Boris Brezillon wrote: > Le Sat, 17 Jun 2017 00:46:12 +0200, > Peter Rosin <peda@axentia.se> a écrit : > >>>>>> Hm, it's probably too late to do it here. Planes have already been >>>>>> enabled and the engine may have started to fetch data and do the >>>>>> composition. You could do that in ->update_plane() [1], and make it a >>>>>> per-plane thing. >>>>>> >>>>>> I'm not sure, but I think you can get the new crtc_state from >>>>>> plane->crtc->state in this context (state have already been swapped, >>>>>> and new state is being applied, which means relevant locks are held). >>>>> >>>>> Ok, I can move it there. My plan is to just copy the default .update_plane >>>>> function and insert >>>>> >>>>> if (crtc->state->color_mgmt_changed && crtc->state->gamma_lut) { >>>>> ... >>>>> } >>>>> >>>>> just before the drm_atomic_commit(state) call. Sounds ok? >>>> >>>> Why would you copy the default ->update_plane() when we already have >>>> our own ->atomic_update_plane() implementation [1]? Just put it there >>>> (before the atmel_hlcdc_layer_update_commit() call) and we should be >>>> good. >>> >>> Ahh, but you said ->update_plane() and I took that as .update_plane in >>> layer_plane_funcs, not ->atomic_update() in atmel_hlcdc_layer_plane_helper_funcs. >>> >>> Makes sense now, and much neater too. >> >> No, it doesn't make sense. There's no atmel_hlcdc_layer_update_commit call >> anywhere, and no such function. You seem to have some further changes that >> are not even in -next. Where am I getting those changes and why are they >> not upstream yet? > > My bad, this part as been reworked in 4.12 and I was reading 4.11 code. > Indeed, atmel_hlcdc_layer_update_commit() no longer exists, but > atmel_hlcdc_plane_atomic_update() does. Ah, it was the other way around. I had too new code! Anyway, I just sent a new series which I think addresses all issues except that I have still not tested with plain drm ioctls. Cheers, peda
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c index 5348985..75871b5 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c @@ -61,6 +61,7 @@ struct atmel_hlcdc_crtc { struct atmel_hlcdc_dc *dc; struct drm_pending_vblank_event *event; int id; + u32 clut[ATMEL_HLCDC_CLUT_SIZE]; }; static inline struct atmel_hlcdc_crtc * @@ -140,6 +141,46 @@ static void atmel_hlcdc_crtc_mode_set_nofb(struct drm_crtc *c) cfg); } +static void +atmel_hlcdc_crtc_load_lut(struct drm_crtc *c) +{ + struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c); + struct atmel_hlcdc_dc *dc = crtc->dc; + int layer; + int idx; + + for (layer = 0; layer < ATMEL_HLCDC_MAX_LAYERS; layer++) { + if (!dc->layers[layer]) + continue; + for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) + atmel_hlcdc_layer_write_clut(dc->layers[layer], + idx, crtc->clut[idx]); + } +} + +static void +atmel_hlcdc_crtc_flush_lut(struct drm_crtc *c) +{ + struct atmel_hlcdc_crtc *crtc = drm_crtc_to_atmel_hlcdc_crtc(c); + struct drm_crtc_state *state = c->state; + struct drm_color_lut *lut; + int idx; + + if (!state->gamma_lut) + return; + + lut = (struct drm_color_lut *)state->gamma_lut->data; + + for (idx = 0; idx < ATMEL_HLCDC_CLUT_SIZE; idx++) { + crtc->clut[idx] = + ((lut[idx].red << 8) & 0xff0000) | + (lut[idx].green & 0xff00) | + (lut[idx].blue >> 8); + } + + atmel_hlcdc_crtc_load_lut(c); +} + static enum drm_mode_status atmel_hlcdc_crtc_mode_valid(struct drm_crtc *c, const struct drm_display_mode *mode) @@ -312,6 +353,9 @@ static void atmel_hlcdc_crtc_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_s) { /* TODO: write common plane control register if available */ + + if (crtc->state->color_mgmt_changed) + atmel_hlcdc_crtc_flush_lut(crtc); } static const struct drm_crtc_helper_funcs lcdc_crtc_helper_funcs = { @@ -429,6 +473,7 @@ static const struct drm_crtc_funcs atmel_hlcdc_crtc_funcs = { .atomic_destroy_state = atmel_hlcdc_crtc_destroy_state, .enable_vblank = atmel_hlcdc_crtc_enable_vblank, .disable_vblank = atmel_hlcdc_crtc_disable_vblank, + .set_property = drm_atomic_helper_crtc_set_property, }; int atmel_hlcdc_crtc_create(struct drm_device *dev) @@ -484,6 +529,9 @@ int atmel_hlcdc_crtc_create(struct drm_device *dev) drm_crtc_helper_add(&crtc->base, &lcdc_crtc_helper_funcs); drm_crtc_vblank_reset(&crtc->base); + drm_mode_crtc_set_gamma_size(&crtc->base, ATMEL_HLCDC_CLUT_SIZE); + drm_crtc_enable_color_mgmt(&crtc->base, 0, false, ATMEL_HLCDC_CLUT_SIZE); + dc->crtc = &crtc->base; return 0; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c index 30dbffd..4f6ef07 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c @@ -42,6 +42,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9n12_layers[] = { .default_color = 3, .general_config = 4, }, + .clut_offset = 0x400, }, }; @@ -73,6 +74,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = { .disc_pos = 5, .disc_size = 6, }, + .clut_offset = 0x400, }, { .name = "overlay1", @@ -91,6 +93,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = { .chroma_key_mask = 8, .general_config = 9, }, + .clut_offset = 0x800, }, { .name = "high-end-overlay", @@ -112,6 +115,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = { .scaler_config = 13, .csc = 14, }, + .clut_offset = 0x1000, }, { .name = "cursor", @@ -131,6 +135,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_at91sam9x5_layers[] = { .chroma_key_mask = 8, .general_config = 9, }, + .clut_offset = 0x1400, }, }; @@ -162,6 +167,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = { .disc_pos = 5, .disc_size = 6, }, + .clut_offset = 0x600, }, { .name = "overlay1", @@ -180,6 +186,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = { .chroma_key_mask = 8, .general_config = 9, }, + .clut_offset = 0xa00, }, { .name = "overlay2", @@ -198,6 +205,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = { .chroma_key_mask = 8, .general_config = 9, }, + .clut_offset = 0xe00, }, { .name = "high-end-overlay", @@ -223,6 +231,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = { }, .csc = 14, }, + .clut_offset = 0x1200, }, { .name = "cursor", @@ -244,6 +253,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d3_layers[] = { .general_config = 9, .scaler_config = 13, }, + .clut_offset = 0x1600, }, }; @@ -275,6 +285,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = { .disc_pos = 5, .disc_size = 6, }, + .clut_offset = 0x600, }, { .name = "overlay1", @@ -293,6 +304,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = { .chroma_key_mask = 8, .general_config = 9, }, + .clut_offset = 0xa00, }, { .name = "overlay2", @@ -336,6 +348,7 @@ static const struct atmel_hlcdc_layer_desc atmel_hlcdc_sama5d4_layers[] = { }, .csc = 14, }, + .clut_offset = 0x1200, }, }; diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h index b0596a8..709f7b9 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h @@ -88,6 +88,11 @@ #define ATMEL_HLCDC_YUV422SWP BIT(17) #define ATMEL_HLCDC_DSCALEOPT BIT(20) +#define ATMEL_HLCDC_C1_MODE ATMEL_HLCDC_CLUT_MODE(0) +#define ATMEL_HLCDC_C2_MODE ATMEL_HLCDC_CLUT_MODE(1) +#define ATMEL_HLCDC_C4_MODE ATMEL_HLCDC_CLUT_MODE(2) +#define ATMEL_HLCDC_C8_MODE ATMEL_HLCDC_CLUT_MODE(3) + #define ATMEL_HLCDC_XRGB4444_MODE ATMEL_HLCDC_RGB_MODE(0) #define ATMEL_HLCDC_ARGB4444_MODE ATMEL_HLCDC_RGB_MODE(1) #define ATMEL_HLCDC_RGBA4444_MODE ATMEL_HLCDC_RGB_MODE(2) @@ -142,6 +147,8 @@ #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_DONE BIT(2) #define ATMEL_HLCDC_DMA_CHANNEL_DSCR_OVERRUN BIT(3) +#define ATMEL_HLCDC_CLUT_SIZE 256 + #define ATMEL_HLCDC_MAX_LAYERS 6 /** @@ -259,6 +266,7 @@ struct atmel_hlcdc_layer_desc { int id; int regs_offset; int cfgs_offset; + int clut_offset; struct atmel_hlcdc_formats *formats; struct atmel_hlcdc_layer_cfg_layout layout; int max_width; @@ -414,6 +422,14 @@ static inline u32 atmel_hlcdc_layer_read_cfg(struct atmel_hlcdc_layer *layer, (cfgid * sizeof(u32))); } +static inline void atmel_hlcdc_layer_write_clut(struct atmel_hlcdc_layer *layer, + unsigned int c, u32 val) +{ + atmel_hlcdc_layer_write_reg(layer, + layer->desc->clut_offset + c * sizeof(u32), + val); +} + static inline void atmel_hlcdc_layer_init(struct atmel_hlcdc_layer *layer, const struct atmel_hlcdc_layer_desc *desc, struct regmap *regmap) diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c index 1124200..5537843 100644 --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c @@ -83,6 +83,7 @@ drm_plane_state_to_atmel_hlcdc_plane_state(struct drm_plane_state *s) #define SUBPIXEL_MASK 0xffff static uint32_t rgb_formats[] = { + DRM_FORMAT_C8, DRM_FORMAT_XRGB4444, DRM_FORMAT_ARGB4444, DRM_FORMAT_RGBA4444, @@ -100,6 +101,7 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_formats = { }; static uint32_t rgb_and_yuv_formats[] = { + DRM_FORMAT_C8, DRM_FORMAT_XRGB4444, DRM_FORMAT_ARGB4444, DRM_FORMAT_RGBA4444, @@ -128,6 +130,9 @@ struct atmel_hlcdc_formats atmel_hlcdc_plane_rgb_and_yuv_formats = { static int atmel_hlcdc_format_to_plane_mode(u32 format, u32 *mode) { switch (format) { + case DRM_FORMAT_C8: + *mode = ATMEL_HLCDC_C8_MODE; + break; case DRM_FORMAT_XRGB4444: *mode = ATMEL_HLCDC_XRGB4444_MODE; break;
All layers of chips support this, the only variable is the base address of the lookup table in the register map. Signed-off-by: Peter Rosin <peda@axentia.se> --- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 48 +++++++++++++++++++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 13 +++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 16 +++++++++ drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 5 +++ 4 files changed, 82 insertions(+)