Re: [PATCH v2] mgag200: Enable atomic gamma lut update

2022-05-12 Thread Jocelyn Falempe

On 12/05/2022 10:52, Thomas Zimmermann wrote:

Hi

Am 11.05.22 um 17:28 schrieb Jocelyn Falempe:

Add support for atomic update of gamma lut.
With this patch the "Night light" feature of gnome3
is working properly on mgag200.

v2:
  - Add a default linear gamma function
  - renamed functions with mgag200 prefix
  - use format's 4cc code instead of bit depth
  - use better interpolation for 16bits gamma
  - remove legacy function mga_crtc_load_lut()
  - can't remove the call to drm_mode_crtc_set_gamma_size()
 because it doesn't work with userspace.
  - other small refactors

Signed-off-by: Jocelyn Falempe 


I already gave a Tested-by on the first iteration. It's good practice to 
add these tags in follow-up patches unless the patch has changed entirely.


Sorry, I though I changed too much code in v2 to add it.


A few more comments are below. With those fixed:

Reviewed-by: Thomas Zimmermann 

I suggest to post another version of the patch and merge it if no 
further comments arrive within 2 days.



---
  drivers/gpu/drm/mgag200/mgag200_mode.c | 125 -
  1 file changed, 81 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c

index 6e18d3bbd720..b748bc5b0e93 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -32,57 +32,76 @@
   * This file contains setup code for the CRTC.
   */
-static void mga_crtc_load_lut(struct drm_crtc *crtc)
+static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
+  uint32_t format)
  {
-    struct drm_device *dev = crtc->dev;
-    struct mga_device *mdev = to_mga_device(dev);
-    struct drm_framebuffer *fb;
-    u16 *r_ptr, *g_ptr, *b_ptr;
  int i;
-    if (!crtc->enabled)
-    return;
-
-    if (!mdev->display_pipe.plane.state)
-    return;
+    WREG8(DAC_INDEX + MGA1064_INDEX, 0);
-    fb = mdev->display_pipe.plane.state->fb;
+    switch (format) {
+    case DRM_FORMAT_RGB565:
+    /* Use better interpolation, to take 32 values from 0 to 255 */
+    for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+    }
+    /* Green has one more bit, so add padding with 0 for red and 
blue. */

+    for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+    }
+    break;
+    case DRM_FORMAT_RGB888:
+    case DRM_FORMAT_XRGB:
+    for (i = 0; i < MGAG200_LUT_SIZE; i++) {
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+    }


These loops look much nicer to me.


+    break;
+    default:
+    drm_warn_once(>base, "Unsupported format for gamma 
%d\n", format);


There's a print format modifier for 4cc formats. It's %p4cc and expects 
a pointer to the format's 4cc code. See 'git grep p4cc' for examples.


ok, that's a cool feature.



The comment itself is not easy to understand. Maybe "Unsupported format 
%p4cc for gamma correction.\n" ?


Sure, having good error message is always helpful.




+    break;
+    }
+}
-    r_ptr = crtc->gamma_store;
-    g_ptr = r_ptr + crtc->gamma_size;
-    b_ptr = g_ptr + crtc->gamma_size;
+static void mgag200_crtc_set_gamma(struct mga_device *mdev,
+   struct drm_color_lut *lut,
+   uint32_t format)
+{
+    int i;
  WREG8(DAC_INDEX + MGA1064_INDEX, 0);
-    if (fb && fb->format->cpp[0] * 8 == 16) {
-    int inc = (fb->format->depth == 15) ? 8 : 4;
-    u8 r, b;
-    for (i = 0; i < MGAG200_LUT_SIZE; i += inc) {
-    if (fb->format->depth == 16) {
-    if (i > (MGAG200_LUT_SIZE >> 1)) {
-    r = b = 0;
-    } else {
-    r = *r_ptr++ >> 8;
-    b = *b_ptr++ >> 8;
-    r_ptr++;
-    b_ptr++;
-    }
-    } else {
-    r = *r_ptr++ >> 8;
-    b = *b_ptr++ >> 8;
-    }
-    /* VGA registers */
-    WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
-    WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
-    WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
+    switch (format) {
+    case DRM_FORMAT_RGB565:
+    /* Use better interpolation, to take 32 values from lut[0] to 
lut[255] */

+    for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 4].red 
>> 8);
+    WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i / 
16].green >> 8);
+    WREG8(DAC_INDEX + 

Re: [PATCH v2] mgag200: Enable atomic gamma lut update

2022-05-12 Thread Thomas Zimmermann

Hi

Am 11.05.22 um 17:28 schrieb Jocelyn Falempe:

Add support for atomic update of gamma lut.
With this patch the "Night light" feature of gnome3
is working properly on mgag200.

v2:
  - Add a default linear gamma function
  - renamed functions with mgag200 prefix
  - use format's 4cc code instead of bit depth
  - use better interpolation for 16bits gamma
  - remove legacy function mga_crtc_load_lut()
  - can't remove the call to drm_mode_crtc_set_gamma_size()
 because it doesn't work with userspace.
  - other small refactors

Signed-off-by: Jocelyn Falempe 


I already gave a Tested-by on the first iteration. It's good practice to 
add these tags in follow-up patches unless the patch has changed entirely.


A few more comments are below. With those fixed:

Reviewed-by: Thomas Zimmermann 

I suggest to post another version of the patch and merge it if no 
further comments arrive within 2 days.



---
  drivers/gpu/drm/mgag200/mgag200_mode.c | 125 -
  1 file changed, 81 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6e18d3bbd720..b748bc5b0e93 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -32,57 +32,76 @@
   * This file contains setup code for the CRTC.
   */
  
-static void mga_crtc_load_lut(struct drm_crtc *crtc)

+static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
+ uint32_t format)
  {
-   struct drm_device *dev = crtc->dev;
-   struct mga_device *mdev = to_mga_device(dev);
-   struct drm_framebuffer *fb;
-   u16 *r_ptr, *g_ptr, *b_ptr;
int i;
  
-	if (!crtc->enabled)

-   return;
-
-   if (!mdev->display_pipe.plane.state)
-   return;
+   WREG8(DAC_INDEX + MGA1064_INDEX, 0);
  
-	fb = mdev->display_pipe.plane.state->fb;

+   switch (format) {
+   case DRM_FORMAT_RGB565:
+   /* Use better interpolation, to take 32 values from 0 to 255 */
+   for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+   }
+   /* Green has one more bit, so add padding with 0 for red and 
blue. */
+   for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+   }
+   break;
+   case DRM_FORMAT_RGB888:
+   case DRM_FORMAT_XRGB:
+   for (i = 0; i < MGAG200_LUT_SIZE; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+   }


These loops look much nicer to me.


+   break;
+   default:
+   drm_warn_once(>base, "Unsupported format for gamma %d\n", 
format);


There's a print format modifier for 4cc formats. It's %p4cc and expects 
a pointer to the format's 4cc code. See 'git grep p4cc' for examples.


The comment itself is not easy to understand. Maybe "Unsupported format 
%p4cc for gamma correction.\n" ?



+   break;
+   }
+}
  
-	r_ptr = crtc->gamma_store;

-   g_ptr = r_ptr + crtc->gamma_size;
-   b_ptr = g_ptr + crtc->gamma_size;
+static void mgag200_crtc_set_gamma(struct mga_device *mdev,
+  struct drm_color_lut *lut,
+  uint32_t format)
+{
+   int i;
  
  	WREG8(DAC_INDEX + MGA1064_INDEX, 0);
  
-	if (fb && fb->format->cpp[0] * 8 == 16) {

-   int inc = (fb->format->depth == 15) ? 8 : 4;
-   u8 r, b;
-   for (i = 0; i < MGAG200_LUT_SIZE; i += inc) {
-   if (fb->format->depth == 16) {
-   if (i > (MGAG200_LUT_SIZE >> 1)) {
-   r = b = 0;
-   } else {
-   r = *r_ptr++ >> 8;
-   b = *b_ptr++ >> 8;
-   r_ptr++;
-   b_ptr++;
-   }
-   } else {
-   r = *r_ptr++ >> 8;
-   b = *b_ptr++ >> 8;
-   }
-   /* VGA registers */
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
+   

Re: [PATCH v2] mgag200: Enable atomic gamma lut update

2022-05-11 Thread kernel test robot
Hi Jocelyn,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tegra-drm/drm/tegra/for-next]
[also build test WARNING on v5.18-rc6]
[cannot apply to drm/drm-next drm-tip/drm-tip airlied/drm-next next-20220511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/mgag200-Enable-atomic-gamma-lut-update/20220511-233134
base:   git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next
config: i386-randconfig-a003-20220509 
(https://download.01.org/0day-ci/archive/20220512/202205120649.u2ym0pxz-...@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 
18dd123c56754edf62c7042dcf23185c3727610f)
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/intel-lab-lkp/linux/commit/0831f1db9ae8814796efea603749709e80d2808c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Jocelyn-Falempe/mgag200-Enable-atomic-gamma-lut-update/20220511-233134
git checkout 0831f1db9ae8814796efea603749709e80d2808c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 
O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/mgag200/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/mgag200/mgag200_mode.c:972:5: warning: format specifies type 
>> 'long' but the argument has type 'size_t' (aka 'unsigned int') [-Wformat]
   crtc_state->gamma_lut->length);
   ^
   include/drm/drm_print.h:438:46: note: expanded from macro 'drm_err'
   __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__)
~~~^~~
   include/drm/drm_print.h:425:48: note: expanded from macro '__drm_printk'
   dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
  ~~~^~~
   include/linux/dev_printk.h:144:65: note: expanded from macro 'dev_err'
   dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), 
##__VA_ARGS__)
  ~~~ 
^~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 
'dev_printk_index_wrap'
   _p_func(dev, fmt, ##__VA_ARGS__);   \
~~~^~~
   1 warning generated.


vim +972 drivers/gpu/drm/mgag200/mgag200_mode.c

   937  
   938  static int
   939  mgag200_simple_display_pipe_check(struct drm_simple_display_pipe *pipe,
   940struct drm_plane_state *plane_state,
   941struct drm_crtc_state *crtc_state)
   942  {
   943  struct drm_plane *plane = plane_state->plane;
   944  struct drm_device *dev = plane->dev;
   945  struct mga_device *mdev = to_mga_device(dev);
   946  struct mgag200_pll *pixpll = >pixpll;
   947  struct mgag200_crtc_state *mgag200_crtc_state = 
to_mgag200_crtc_state(crtc_state);
   948  struct drm_framebuffer *new_fb = plane_state->fb;
   949  struct drm_framebuffer *fb = NULL;
   950  int ret;
   951  
   952  if (!new_fb)
   953  return 0;
   954  
   955  if (plane->state)
   956  fb = plane->state->fb;
   957  
   958  if (!fb || (fb->format != new_fb->format))
   959  crtc_state->mode_changed = true; /* update PLL settings 
*/
   960  
   961  if (crtc_state->mode_changed) {
   962  ret = pixpll->funcs->compute(pixpll, 
crtc_state->mode.clock,
   963   
_crtc_state->pixpllc);
   964  if (ret)
   965  return ret;
   966  }
   967  
   968  if (crtc_state->color_mgmt_changed && crtc_state->gamma_lut) {
   969  if (crtc_state->gamma_lut->length !=
   970  MGAG200_LUT_SIZE * sizeof(struct drm_color_lut)) {
   971  drm_err(dev, "Wrong size for gamma_lut %ld\n",
 > 972  crtc_state->gamma_lut->length);
   973  return -EINVAL;
   974  }
   975  }
   976  return 0;
   977  }
   978  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp


Re: [PATCH v2] mgag200: Enable atomic gamma lut update

2022-05-11 Thread kernel test robot
Hi Jocelyn,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on tegra-drm/drm/tegra/for-next]
[also build test WARNING on v5.18-rc6]
[cannot apply to drm/drm-next drm-tip/drm-tip airlied/drm-next next-20220511]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/intel-lab-lkp/linux/commits/Jocelyn-Falempe/mgag200-Enable-atomic-gamma-lut-update/20220511-233134
base:   git://anongit.freedesktop.org/tegra/linux.git drm/tegra/for-next
config: arc-allyesconfig 
(https://download.01.org/0day-ci/archive/20220512/202205120525.drseu95x-...@intel.com/config)
compiler: arceb-elf-gcc (GCC) 11.3.0
reproduce (this is a W=1 build):
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# 
https://github.com/intel-lab-lkp/linux/commit/0831f1db9ae8814796efea603749709e80d2808c
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Jocelyn-Falempe/mgag200-Enable-atomic-gamma-lut-update/20220511-233134
git checkout 0831f1db9ae8814796efea603749709e80d2808c
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.3.0 make.cross W=1 
O=build_dir ARCH=arc SHELL=/bin/bash drivers/gpu/drm/mgag200/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
from include/linux/acpi.h:15,
from include/linux/i2c.h:13,
from include/drm/drm_crtc.h:28,
from include/drm/drm_atomic_helper.h:31,
from drivers/gpu/drm/mgag200/mgag200_mode.c:14:
   drivers/gpu/drm/mgag200/mgag200_mode.c: In function 
'mgag200_simple_display_pipe_check':
>> include/drm/drm_print.h:425:39: warning: format '%ld' expects argument of 
>> type 'long int', but argument 3 has type 'size_t' {aka 'unsigned int'} 
>> [-Wformat=]
 425 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
 |   ^~~~
   include/linux/dev_printk.h:110:30: note: in definition of macro 
'dev_printk_index_wrap'
 110 | _p_func(dev, fmt, ##__VA_ARGS__);
   \
 |  ^~~
   include/linux/dev_printk.h:144:56: note: in expansion of macro 'dev_fmt'
 144 | dev_printk_index_wrap(_dev_err, KERN_ERR, dev, dev_fmt(fmt), 
##__VA_ARGS__)
 |^~~
   include/drm/drm_print.h:425:9: note: in expansion of macro 'dev_err'
 425 | dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
 | ^~~~
   include/drm/drm_print.h:438:9: note: in expansion of macro '__drm_printk'
 438 | __drm_printk((drm), err,, "*ERROR* " fmt, ##__VA_ARGS__)
 | ^~~~
   drivers/gpu/drm/mgag200/mgag200_mode.c:971:25: note: in expansion of macro 
'drm_err'
 971 | drm_err(dev, "Wrong size for gamma_lut 
%ld\n",
 | ^~~


vim +425 include/drm/drm_print.h

02c9656b2f0d69 Haneen Mohammed   2017-10-17  385  
02c9656b2f0d69 Haneen Mohammed   2017-10-17  386  /**
b52817e9de06a3 Mauro Carvalho Chehab 2020-10-27  387   * DRM_DEV_DEBUG() - 
Debug output for generic drm code
02c9656b2f0d69 Haneen Mohammed   2017-10-17  388   *
306589856399e1 Douglas Anderson  2021-09-21  389   * NOTE: this is 
deprecated in favor of drm_dbg_core().
306589856399e1 Douglas Anderson  2021-09-21  390   *
091756bbb1a961 Haneen Mohammed   2017-10-17  391   * @dev: device pointer
091756bbb1a961 Haneen Mohammed   2017-10-17  392   * @fmt: printf() like 
format string.
02c9656b2f0d69 Haneen Mohammed   2017-10-17  393   */
db87086492581c Joe Perches   2018-03-16  394  #define 
DRM_DEV_DEBUG(dev, fmt, ...)  \
db87086492581c Joe Perches   2018-03-16  395drm_dev_dbg(dev, 
DRM_UT_CORE, fmt, ##__VA_ARGS__)
b52817e9de06a3 Mauro Carvalho Chehab 2020-10-27  396  /**
b52817e9de06a3 Mauro Carvalho Chehab 2020-10-27  397   * DRM_DEV_DEBUG_DRIVER() 
- Debug output for vendor specific part of the driver
b52817e9de06a3 Mauro Carvalho Chehab 2020-10-27  398   *
306589856399e1 Douglas Anderson  2021-09-21  399   * NOTE: this is 
deprecated in favor of drm_dbg() or dev_dbg().
306589856399e1 Douglas Anderson  2021-09-21  400   *
b52817e9de06a3 Mauro Carvalho Chehab 2020-10-27  401   * @dev: device pointer
b52817e9de06a3 Mauro Carvalho Chehab 2020-10-27  402   * @fmt: printf() like 
format string.

[PATCH v2] mgag200: Enable atomic gamma lut update

2022-05-11 Thread Jocelyn Falempe
Add support for atomic update of gamma lut.
With this patch the "Night light" feature of gnome3
is working properly on mgag200.

v2:
 - Add a default linear gamma function
 - renamed functions with mgag200 prefix
 - use format's 4cc code instead of bit depth
 - use better interpolation for 16bits gamma
 - remove legacy function mga_crtc_load_lut()
 - can't remove the call to drm_mode_crtc_set_gamma_size()
because it doesn't work with userspace.
 - other small refactors

Signed-off-by: Jocelyn Falempe 
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 125 -
 1 file changed, 81 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6e18d3bbd720..b748bc5b0e93 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -32,57 +32,76 @@
  * This file contains setup code for the CRTC.
  */
 
-static void mga_crtc_load_lut(struct drm_crtc *crtc)
+static void mgag200_crtc_set_gamma_linear(struct mga_device *mdev,
+ uint32_t format)
 {
-   struct drm_device *dev = crtc->dev;
-   struct mga_device *mdev = to_mga_device(dev);
-   struct drm_framebuffer *fb;
-   u16 *r_ptr, *g_ptr, *b_ptr;
int i;
 
-   if (!crtc->enabled)
-   return;
-
-   if (!mdev->display_pipe.plane.state)
-   return;
+   WREG8(DAC_INDEX + MGA1064_INDEX, 0);
 
-   fb = mdev->display_pipe.plane.state->fb;
+   switch (format) {
+   case DRM_FORMAT_RGB565:
+   /* Use better interpolation, to take 32 values from 0 to 255 */
+   for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 8 + i / 4);
+   }
+   /* Green has one more bit, so add padding with 0 for red and 
blue. */
+   for (i = MGAG200_LUT_SIZE / 8; i < MGAG200_LUT_SIZE / 4; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i * 4 + i / 16);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, 0);
+   }
+   break;
+   case DRM_FORMAT_RGB888:
+   case DRM_FORMAT_XRGB:
+   for (i = 0; i < MGAG200_LUT_SIZE; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, i);
+   }
+   break;
+   default:
+   drm_warn_once(>base, "Unsupported format for gamma %d\n", 
format);
+   break;
+   }
+}
 
-   r_ptr = crtc->gamma_store;
-   g_ptr = r_ptr + crtc->gamma_size;
-   b_ptr = g_ptr + crtc->gamma_size;
+static void mgag200_crtc_set_gamma(struct mga_device *mdev,
+  struct drm_color_lut *lut,
+  uint32_t format)
+{
+   int i;
 
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
 
-   if (fb && fb->format->cpp[0] * 8 == 16) {
-   int inc = (fb->format->depth == 15) ? 8 : 4;
-   u8 r, b;
-   for (i = 0; i < MGAG200_LUT_SIZE; i += inc) {
-   if (fb->format->depth == 16) {
-   if (i > (MGAG200_LUT_SIZE >> 1)) {
-   r = b = 0;
-   } else {
-   r = *r_ptr++ >> 8;
-   b = *b_ptr++ >> 8;
-   r_ptr++;
-   b_ptr++;
-   }
-   } else {
-   r = *r_ptr++ >> 8;
-   b = *b_ptr++ >> 8;
-   }
-   /* VGA registers */
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
+   switch (format) {
+   case DRM_FORMAT_RGB565:
+   /* Use better interpolation, to take 32 values from lut[0] to 
lut[255] */
+   for (i = 0; i < MGAG200_LUT_SIZE / 8; i++) {
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 
4].red >> 8);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 4 + i / 
16].green >> 8);
+   WREG8(DAC_INDEX + MGA1064_COL_PAL, lut[i * 8 + i / 
4].blue >> 8);
}
-   return;
-   }
-   for (i = 0; i < MGAG200_LUT_SIZE; i++) {
-   /* VGA registers */
-   WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
-