search for: gamma_get

This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get totally obsolete. I think the gamma_store can end up invalid on error. But the way I read it, that can happen in drm_mode_gamma_set_ioctl as well, so why should this pesky legacy fbdev stuff be any better? drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However, it saves it to...

...This callback is functionally redundant with the core gamma table - * support and simply exists because the fbdev hasn't yet been - * refactored to use the core gamma table interfaces. - */ - void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green, - u16 blue, int regno); - /** - * @gamma_get: - * - * Read the given gamma LUT register on the given CRTC, used to save the - * current LUT when force-restoring the fbdev for e.g. kdbg. - * - * This callback is optional. - * - * FIXME: - * - * This callback is functionally redundant with the core gamma table - * support and simply e...

On 2017-06-21 09:38, Daniel Vetter wrote: > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get >> totally obsolete. >> >> I think the gamma_store can end up invalid on error. But the way I read >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should >> this pesky legacy fbdev stuff be any better? >> >> drm_fb_helper_save_lut_atomic...

Hi! While trying to get CLUT support for the atmel_hlcdc driver, and specifically for the emulated fbdev interface, I received some push-back that my feeble in-driver attempts should be solved by the core. This is my attempt to do it right. Boris and Daniel, was this approximately what you had in mind? I have obviously not tested all of this with more than a compile, but the first patch is

...ma_set = armada_drm_crtc_gamma_set, > ^~~~~~~~~~~~~~~~~~~~~~~~~ > drivers/gpu//drm/armada/armada_fbdev.c:121:15: note: (near initialization for 'armada_fb_helper_funcs.fb_probe') > >> drivers/gpu//drm/armada/armada_fbdev.c:122:2: error: unknown field 'gamma_get' specified in initializer > .gamma_get = armada_drm_crtc_gamma_get, > ^ > drivers/gpu//drm/armada/armada_fbdev.c:122:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] > .gamma_get = armada_drm_crtc_gamma_get, >...

...surprised if I have fouled up some bit-manipulation somewhere in this mostly mechanical change... Changes since v1: - Rebased to next-20170621 - Split 1/11 into a preparatory patch, a cleanup patch and then the meat in 3/14. - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. - Removed the empty .gamma_get/.gamma_set fb helpers from the armada driver that I had somehow managed to ignore but which 0day found real quick. - Be less judgemental on drivers only providing .gamma_get and .gamma_set, but no .load_lut. That's actually a valid thing to do if you only need pseudo-palette for FB_VISU...

On 2017-06-22 13:49, Philippe CORNU wrote: > On 06/22/2017 08:06 AM, Peter Rosin wrote: >> The redundant fb helper .load_lut is no longer used, and can not >> work right without also providing the fb helpers .gamma_set and >> .gamma_get thus rendering the code in this driver suspect. >> > > Hi Peter, > STM32 chipsets supports 8-bit CLUT mode but this driver version does not > support it "yet" (final patch has not been upstreamed because it was a > too big fbdev patch for simply adding CLUT...). &g...

On 21/06/17 04:38 PM, Daniel Vetter wrote: > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get >> totally obsolete. >> >> I think the gamma_store can end up invalid on error. But the way I read >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should >> this pesky legacy fbdev stuff be any better? >> >> drm_fb_helper_save_lut_atomic...

On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: > This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get > totally obsolete. > > I think the gamma_store can end up invalid on error. But the way I read > it, that can happen in drm_mode_gamma_set_ioctl as well, so why should > this pesky legacy fbdev stuff be any better? > > drm_fb_helper_save_lut_atomic justs saves the gamma lut...

...ypes] .gamma_set = armada_drm_crtc_gamma_set, ^~~~~~~~~~~~~~~~~~~~~~~~~ drivers/gpu//drm/armada/armada_fbdev.c:121:15: note: (near initialization for 'armada_fb_helper_funcs.fb_probe') >> drivers/gpu//drm/armada/armada_fbdev.c:122:2: error: unknown field 'gamma_get' specified in initializer .gamma_get = armada_drm_crtc_gamma_get, ^ drivers/gpu//drm/armada/armada_fbdev.c:122:15: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types] .gamma_get = armada_drm_crtc_gamma_get, ^~~~~~~~~~~~~~...

On 06/22/2017 08:06 AM, Peter Rosin wrote: > The redundant fb helper .load_lut is no longer used, and can not > work right without also providing the fb helpers .gamma_set and > .gamma_get thus rendering the code in this driver suspect. > Hi Peter, STM32 chipsets supports 8-bit CLUT mode but this driver version does not support it "yet" (final patch has not been upstreamed because it was a too big fbdev patch for simply adding CLUT...). Regarding your patch below, i...

...mostly mechanical change... >> >> Changes since v1: >> >> - Rebased to next-20170621 >> - Split 1/11 into a preparatory patch, a cleanup patch and then >> the meat in 3/14. >> - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. >> - Removed the empty .gamma_get/.gamma_set fb helpers from the >> armada driver that I had somehow managed to ignore but which >> 0day found real quick. >> - Be less judgemental on drivers only providing .gamma_get and >> .gamma_set, but no .load_lut. That's actually a valid thing >> to d...

...08:36, Daniel Vetter wrote: > On Wed, Jun 21, 2017 at 11:40:52AM +0200, Peter Rosin wrote: >> On 2017-06-21 09:38, Daniel Vetter wrote: >>> On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: >>>> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get >>>> totally obsolete. >>>> >>>> I think the gamma_store can end up invalid on error. But the way I read >>>> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should >>>> this pesky legacy fbdev stuff be any better? >>...

...to get the full series (or nothing at all) for the next round (if that is needed) just say so. Changes since v1: - Rebased to next-20170621 - Split 1/11 into a preparatory patch, a cleanup patch and then the meat in 3/14. - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. - Removed the empty .gamma_get/.gamma_set fb helpers from the armada driver that I had somehow managed to ignore but which 0day found real quick. - Be less judgemental on drivers only providing .gamma_get and .gamma_set, but no .load_lut. That's actually a valid thing to do if you only need pseudo-palette for FB_VISU...

...to get the full series (or nothing at all) for the next round (if that is needed) just say so. Changes since v1: - Rebased to next-20170621 - Split 1/11 into a preparatory patch, a cleanup patch and then the meat in 3/14. - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. - Removed the empty .gamma_get/.gamma_set fb helpers from the armada driver that I had somehow managed to ignore but which 0day found real quick. - Be less judgemental on drivers only providing .gamma_get and .gamma_set, but no .load_lut. That's actually a valid thing to do if you only need pseudo-palette for FB_VISU...

...manipulation somewhere in this mostly mechanical change... > > Changes since v1: > > - Rebased to next-20170621 > - Split 1/11 into a preparatory patch, a cleanup patch and then > the meat in 3/14. > - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. > - Removed the empty .gamma_get/.gamma_set fb helpers from the > armada driver that I had somehow managed to ignore but which > 0day found real quick. > - Be less judgemental on drivers only providing .gamma_get and > .gamma_set, but no .load_lut. That's actually a valid thing > to do if you only need p...

...patches 1-3, but there's minor conflicts ... -Daniel > > Changes since v1: > > - Rebased to next-20170621 > - Split 1/11 into a preparatory patch, a cleanup patch and then > the meat in 3/14. > - Handle pseudo-palette for FB_VISUAL_TRUECOLOR. > - Removed the empty .gamma_get/.gamma_set fb helpers from the > armada driver that I had somehow managed to ignore but which > 0day found real quick. > - Be less judgemental on drivers only providing .gamma_get and > .gamma_set, but no .load_lut. That's actually a valid thing > to do if you only need p...

On Wed, Jun 21, 2017 at 11:40:52AM +0200, Peter Rosin wrote: > On 2017-06-21 09:38, Daniel Vetter wrote: > > On Tue, Jun 20, 2017 at 09:25:25PM +0200, Peter Rosin wrote: > >> This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get > >> totally obsolete. > >> > >> I think the gamma_store can end up invalid on error. But the way I read > >> it, that can happen in drm_mode_gamma_set_ioctl as well, so why should > >> this pesky legacy fbdev stuff be any better? > >> > >...

The redundant fb helpers .gamma_set and .gamma_get are no longer used. Remove the dead code. Signed-off-by: Peter Rosin <peda at axentia.se> --- drivers/gpu/drm/armada/armada_crtc.c | 10 ---------- drivers/gpu/drm/armada/armada_crtc.h | 2 -- drivers/gpu/drm/armada/armada_fbdev.c | 2 -- 3 files changed, 14 deletions(-) diff --git a/d...

...This callback is functionally redundant with the core gamma table - * support and simply exists because the fbdev hasn't yet been - * refactored to use the core gamma table interfaces. - */ - void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green, - u16 blue, int regno); - /** - * @gamma_get: - * - * Read the given gamma LUT register on the given CRTC, used to save the - * current LUT when force-restoring the fbdev for e.g. kdbg. - * - * This callback is optional. - * - * FIXME: - * - * This callback is functionally redundant with the core gamma table - * support and simply e...