On 2017-06-15 13:54, Boris Brezillon wrote: > On Thu, 15 Jun 2017 13:47:35 +0200 > Peter Rosin <peda@xxxxxxxxxx> wrote: > >> On 2017-06-15 12:15, Boris Brezillon wrote: >>> On Thu, 15 Jun 2017 11:54:29 +0200 >>> Peter Rosin <peda@xxxxxxxxxx> wrote: >>> >>>> On 2017-06-13 17:30, Boris Brezillon wrote: >>>>> Hi Peter, >>>>> >>>>> On Tue, 13 Jun 2017 16:34:25 +0200 >>>>> Peter Rosin <peda@xxxxxxxxxx> wrote: >>>>> >>>>>> Hi! >>>>>> >>>>>> I need color lookup support for the atmel-hlcdc driver, and had a peek >>>>>> at the code. I also looked at the drivers/gpu/drm/stm driver and came >>>>>> up with the below diff. It compiles, but I have not booted it for the >>>>>> simple reason that I can't imagine it will work. >>>>>> >>>>>> Sure, the code fills the clut registers in the .load_lut helper function >>>>>> and I think it might even do the right thing when setting up the mode. >>>>>> I'm less sure DMA will work correctly? It might, but I haven't looked at >>>>>> it for many seconds... >>>>>> >>>>>> The big question is how the color info gets into the new clut array >>>>>> I created in atmel_hlcdc_crtc? When I look at the stm driver, which does >>>>>> it just like this AFAICT I just don't see it. So, what I'm I missing? >>>>> >>>>> You should look at drm_atomic_helper_legacy_gamma_set() and its users. >>>> >>>> Ok, thanks. I had a long look and could not get it to work at all. >>> >>> Probably because you're trying to set this up through the emulated >>> fbdev interface. The solution I proposed is supposed to be accessed >>> through DRM ioctls. >> >> Yes, that seems like the root cause of most of my troubles. But, the apps >> I have seem to use the fbdev interface, and if the emulation is poorly >> done, they won't work right... >> >>>> So, >>>> I did as below instead. However, there are a few glaring problems... >>> >>> Well, I doubt this will be accepted. The fbdev emulation layer is >>> supposed to be rather dumb, partly because DRM people want developers >>> to switch to the DRM interface. >> >> Sigh. >> >>> Also note that I won't accept a solution that supports setting the LUT >>> table only through the fbdev interface, so please try to make it work >>> the DRM way before you even consider modifying the fbdev emulation >>> layer. >> >> Ok, I'll have another look. But to me, it's a maze... >> >>>> >>>> Something, somewhere, sets up default entries for the 16 first entries >>>> of the palette and seem to expect them to stay as some kind of safe >>>> basic palette, and my applications don't handle it too well. When they >>>> want to draw black, they get a poisonous cyan/green instead etc. But >>>> it's pretty close. Can anyone provide some clue as to how that is >>>> supposed to be handled? >>>> >>>> Also, I had to change the second argument of the drm_fbdev_cma_init... >>>> call at the end of atmel_hlcdc_dc.c:atmel_hlcdc_dc_load() from 24 to 8 >>>> to make it possible to set 8-modes. However, doing so fucks up 24-bit >>>> modes. Which made me suspect that no non-24-bit mode work w/o my patch. >>>> And I could indeed only get 24-bit modes to work, unless I changed this >>>> value to 16. Then RGB565 works like a charm, but RGB888 don't. What's >>>> up with that? Seems like a rather silly limitation, but it's perhaps >>>> just a bug? >>> >>> I'm pretty sure this is not a bug. AFAIR, prefered_bpp is used when you >>> don't explicitly specify the video mode you want to use on the cmdline >>> [1]. >>> >>> [1]http://elixir.free-electrons.com/linux/v3.11/source/Documentation/fb/modedb.txt >>> >> >> (Why refer to v3.11 docs?) >> >> Let's start with some basics, I tried to add >> >> video=atmel-hclcd-dc:1024x768-16 >> >> and >> >> video=atmel_hclcd_dc:1024x768-16 >> >> to the kernel cmdline of an unpatched linux-next kernel. No dice, I can >> still only access RBG888. I'm obviously in need of some hand-holding... >> >> How am I supposed to get a 16-bit mode? > > You should have a look at [1]. > > [1]https://www.at91.com/linux4sam/bin/view/Linux4SAM/UsingAtmelDRMDriver#Choose_supported_modes > Ahh, it works with video=Unknown-1:1024x768-16 Thanks! So, is this below better? I'll follow-up with a patch I need for the fbdev emulation to work. I can't easily test this newfangled drm stuff, my userspace is oldish, so I don't know if atmel_hlcdc_crtc_atomic_flush() is the correct place to call atmel_hlcdc_crtc_flush_lut and have the palette loaded? What I'm saying is that I have only tested the fbdev emulation, and I still have trouble with the palette colors (poisonous green/cyan instead of black etc). But that may only be an issue with fbdev? 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..68f5e66 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[256]; }; 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 < 256; 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 < 256; 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, 256); + drm_crtc_enable_color_mgmt(&crtc->base, 0, false, 256); + 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..a2cc2ab 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) @@ -259,6 +264,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 +420,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; _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
