Hi all,
Inspired by the color manager work, some prep work cleanup for the legacy gamma code. Two more things I'd like to pull off: - rework the fbdev emulation to use the main gamma interfaces, instead of hand-rolling it's own. - add some helpers to implement legacy gamma in terms of atomic and the new color manager stuff. Probably not too useful for i915.ko, since legacy gamma (which is needed to support C8) and the new color manager stuff use completely different hw blocks.
As usual, review&comments highly welcome.
Thanks, Daniel
Daniel Vetter (10): drm: Initialize a linear gamma table by default drm/fb-helper: Remove dead code in setcolreg drm/armada: Drop fb gamma_set/get functions drm/bochs: Drop fake gamma support drm/cirrus: Drop redundnant gamma size check drm/msm: Nuke dummy gamma_set/get functions drm/virtio: Drop dummy gamma table support drm/imx: Don't set a gamma table size drm/qxl: Don't set a gamma table size drm/tegra: Don't set a gamma table size
drivers/gpu/drm/armada/armada_crtc.c | 10 --------- drivers/gpu/drm/armada/armada_crtc.h | 2 -- drivers/gpu/drm/armada/armada_fbdev.c | 2 -- drivers/gpu/drm/bochs/bochs_fbdev.c | 15 -------------- drivers/gpu/drm/bochs/bochs_kms.c | 7 ------- drivers/gpu/drm/cirrus/cirrus_mode.c | 3 --- drivers/gpu/drm/drm_crtc.c | 13 ++++++++++++ drivers/gpu/drm/drm_fb_helper.c | 33 ++---------------------------- drivers/gpu/drm/gma500/psb_intel_display.c | 7 ------- drivers/gpu/drm/imx/imx-drm-core.c | 4 ---- drivers/gpu/drm/msm/msm_fbdev.c | 14 ------------- drivers/gpu/drm/qxl/qxl_display.c | 1 - drivers/gpu/drm/tegra/dc.c | 1 - drivers/gpu/drm/virtio/virtgpu_display.c | 9 -------- 14 files changed, 15 insertions(+), 106 deletions(-)
Code stolen from gma500.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_crtc.c | 13 +++++++++++++ drivers/gpu/drm/gma500/psb_intel_display.c | 7 ------- 2 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 55ffde5a3a4a..a5bba9f023e9 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5138,6 +5138,9 @@ EXPORT_SYMBOL(drm_mode_connector_attach_encoder); int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc, int gamma_size) { + uint16_t *r_base, *g_base, *b_base; + int i; + crtc->gamma_size = gamma_size;
crtc->gamma_store = kcalloc(gamma_size, sizeof(uint16_t) * 3, @@ -5147,6 +5150,16 @@ int drm_mode_crtc_set_gamma_size(struct drm_crtc *crtc, return -ENOMEM; }
+ r_base = crtc->gamma_store; + g_base = r_base + gamma_size; + b_base = g_base + gamma_size; + for (i = 0; i < gamma_size; i++) { + r_base[i] = i << 8; + g_base[i] = i << 8; + b_base[i] = i << 8; + } + + return 0; } EXPORT_SYMBOL(drm_mode_crtc_set_gamma_size); diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c b/drivers/gpu/drm/gma500/psb_intel_display.c index 398015be87e4..7b6c84925098 100644 --- a/drivers/gpu/drm/gma500/psb_intel_display.c +++ b/drivers/gpu/drm/gma500/psb_intel_display.c @@ -491,7 +491,6 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe, struct drm_psb_private *dev_priv = dev->dev_private; struct gma_crtc *gma_crtc; int i; - uint16_t *r_base, *g_base, *b_base;
/* We allocate a extra array of drm_connector pointers * for fbdev after the crtc */ @@ -519,16 +518,10 @@ void psb_intel_crtc_init(struct drm_device *dev, int pipe, gma_crtc->pipe = pipe; gma_crtc->plane = pipe;
- r_base = gma_crtc->base.gamma_store; - g_base = r_base + 256; - b_base = g_base + 256; for (i = 0; i < 256; i++) { gma_crtc->lut_r[i] = i; gma_crtc->lut_g[i] = i; gma_crtc->lut_b[i] = i; - r_base[i] = i << 8; - g_base[i] = i << 8; - b_base[i] = i << 8;
gma_crtc->lut_adj[i] = 0; }
Op 30-03-16 om 11:51 schreef Daniel Vetter:
Code stolen from gma500.
It would be useful to mention why this is done. :)
But even with this patch it seems the legacy gamma in gamma_get is not very reliable. A failed call to gamma_set could leave wrong values in crtc->gamma_store either by calling the ioctl with lut.blue set to NULL, or call gamma_set and interrupt with a signal. For atomic color management get_gamma won't be accurate at all if mixed with new color management.
~Maarten
On Tue, May 31, 2016 at 03:05:32PM +0200, Maarten Lankhorst wrote:
Op 30-03-16 om 11:51 schreef Daniel Vetter:
Code stolen from gma500.
It would be useful to mention why this is done. :)
But even with this patch it seems the legacy gamma in gamma_get is not very reliable. A failed call to gamma_set could leave wrong values in crtc->gamma_store either by calling the ioctl with lut.blue set to NULL, or call gamma_set and interrupt with a signal. For atomic color management get_gamma won't be accurate at all if mixed with new color management.
Hm, it's just some safety code I found laying around, and figured that looks like a good idea. You think it's worth it with some improved commit message added? -Daniel
Op 31-05-16 om 15:24 schreef Daniel Vetter:
On Tue, May 31, 2016 at 03:05:32PM +0200, Maarten Lankhorst wrote:
Op 30-03-16 om 11:51 schreef Daniel Vetter:
Code stolen from gma500.
It would be useful to mention why this is done. :)
But even with this patch it seems the legacy gamma in gamma_get is not very reliable. A failed call to gamma_set could leave wrong values in crtc->gamma_store either by calling the ioctl with lut.blue set to NULL, or call gamma_set and interrupt with a signal. For atomic color management get_gamma won't be accurate at all if mixed with new color management.
Hm, it's just some safety code I found laying around, and figured that looks like a good idea. You think it's worth it with some improved commit message added?
Might as well. It makes gamma_get ioctl slightly more reliable, but for reliable gamma should probably use crtc properties or atomic. :)
DRM fbdev emulation only supports pallete_color with depth == 8, and truecolor with depth > 8. Handling depth == 16 for palettes is hence dead code, let's remove it.
Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/drm_fb_helper.c | 33 ++------------------------------- 1 file changed, 2 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 855108e6e1bd..39f9b8a41843 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -969,7 +969,6 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green, { struct drm_fb_helper *fb_helper = info->par; struct drm_framebuffer *fb = fb_helper->fb; - int pindex;
if (info->fix.visual == FB_VISUAL_TRUECOLOR) { u32 *palette; @@ -1001,38 +1000,10 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green, !fb_helper->funcs->gamma_get)) return -EINVAL;
- pindex = regno; + WARN_ON(fb->bits_per_pixel != 8);
- if (fb->bits_per_pixel == 16) { - pindex = regno << 3; + fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
- if (fb->depth == 16 && regno > 63) - return -EINVAL; - if (fb->depth == 15 && regno > 31) - return -EINVAL; - - if (fb->depth == 16) { - u16 r, g, b; - int i; - if (regno < 32) { - for (i = 0; i < 8; i++) - fb_helper->funcs->gamma_set(crtc, red, - green, blue, pindex + i); - } - - fb_helper->funcs->gamma_get(crtc, &r, - &g, &b, - pindex >> 1); - - for (i = 0; i < 4; i++) - fb_helper->funcs->gamma_set(crtc, r, - green, b, - (pindex >> 1) + i); - } - } - - if (fb->depth != 16) - fb_helper->funcs->gamma_set(crtc, red, green, blue, pindex); return 0; }
Op 30-03-16 om 11:51 schreef Daniel Vetter:
DRM fbdev emulation only supports pallete_color with depth == 8, and truecolor with depth > 8. Handling depth == 16 for palettes is hence dead code, let's remove it.
Hooray, less chance for failure!
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
On Tue, May 31, 2016 at 03:09:42PM +0200, Maarten Lankhorst wrote:
Op 30-03-16 om 11:51 schreef Daniel Vetter:
DRM fbdev emulation only supports pallete_color with depth == 8, and truecolor with depth > 8. Handling depth == 16 for palettes is hence dead code, let's remove it.
Hooray, less chance for failure!
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Applied to drm-misc, thanks for the review. -Daniel
The fb helper private gamma_set/get functions are only required when the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp unconditionally, so this is just dead code. It also doesn't do anything really. Let's just remove it.
Cc: Russell King rmk+kernel@arm.linux.org.uk Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- 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/drivers/gpu/drm/armada/armada_crtc.c b/drivers/gpu/drm/armada/armada_crtc.c index 0293eb74d777..f2979655e100 100644 --- a/drivers/gpu/drm/armada/armada_crtc.c +++ b/drivers/gpu/drm/armada/armada_crtc.c @@ -317,16 +317,6 @@ static void armada_drm_vblank_off(struct armada_crtc *dcrtc) armada_drm_plane_work_run(dcrtc, plane); }
-void armada_drm_crtc_gamma_set(struct drm_crtc *crtc, u16 r, u16 g, u16 b, - int idx) -{ -} - -void armada_drm_crtc_gamma_get(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, - int idx) -{ -} - /* The mode_config.mutex will be held for this call */ static void armada_drm_crtc_dpms(struct drm_crtc *crtc, int dpms) { diff --git a/drivers/gpu/drm/armada/armada_crtc.h b/drivers/gpu/drm/armada/armada_crtc.h index 04fdd22d483b..6e2770343856 100644 --- a/drivers/gpu/drm/armada/armada_crtc.h +++ b/drivers/gpu/drm/armada/armada_crtc.h @@ -92,8 +92,6 @@ struct armada_crtc { }; #define drm_to_armada_crtc(c) container_of(c, struct armada_crtc, crtc)
-void armada_drm_crtc_gamma_set(struct drm_crtc *, u16, u16, u16, int); -void armada_drm_crtc_gamma_get(struct drm_crtc *, u16 *, u16 *, u16 *, int); void armada_drm_crtc_disable_irq(struct armada_crtc *, u32); void armada_drm_crtc_enable_irq(struct armada_crtc *, u32); void armada_drm_crtc_update_regs(struct armada_crtc *, struct armada_regs *); diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c index 7d03c51abcb9..7781bac2ada2 100644 --- a/drivers/gpu/drm/armada/armada_fbdev.c +++ b/drivers/gpu/drm/armada/armada_fbdev.c @@ -124,8 +124,6 @@ static int armada_fb_probe(struct drm_fb_helper *fbh, }
static const struct drm_fb_helper_funcs armada_fb_helper_funcs = { - .gamma_set = armada_drm_crtc_gamma_set, - .gamma_get = armada_drm_crtc_gamma_get, .fb_probe = armada_fb_probe, };
On Wed, Mar 30, 2016 at 11:51:18AM +0200, Daniel Vetter wrote:
The fb helper private gamma_set/get functions are only required when the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp unconditionally, so this is just dead code. It also doesn't do anything really. Let's just remove it.
This comment is misleading: Armada supports 16bpp formats as well as 32bpp, and the hardware does have 8bpp modes to, but I've chosen not to support the 8bpp modes.
On Wed, Mar 30, 2016 at 1:09 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Wed, Mar 30, 2016 at 11:51:18AM +0200, Daniel Vetter wrote:
The fb helper private gamma_set/get functions are only required when the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp unconditionally, so this is just dead code. It also doesn't do anything really. Let's just remove it.
This comment is misleading: Armada supports 16bpp formats as well as 32bpp, and the hardware does have 8bpp modes to, but I've chosen not to support the 8bpp modes.
This is purely about the fbdev emulation (and yeah need to clarify that), not about kms support in general. And these two gamma_set/get hooks are _only_ used by the fbdev emulation, and only needed if you do 8bit paletted mode. Ok if I change the commit message to "Armada used 32bpp unconditionally for fbdev emulation, ..."? -Daniel
On Wed, Mar 30, 2016 at 01:19:21PM +0200, Daniel Vetter wrote:
On Wed, Mar 30, 2016 at 1:09 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Wed, Mar 30, 2016 at 11:51:18AM +0200, Daniel Vetter wrote:
The fb helper private gamma_set/get functions are only required when the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp unconditionally, so this is just dead code. It also doesn't do anything really. Let's just remove it.
This comment is misleading: Armada supports 16bpp formats as well as 32bpp, and the hardware does have 8bpp modes to, but I've chosen not to support the 8bpp modes.
This is purely about the fbdev emulation (and yeah need to clarify that), not about kms support in general. And these two gamma_set/get hooks are _only_ used by the fbdev emulation, and only needed if you do 8bit paletted mode. Ok if I change the commit message to "Armada used 32bpp unconditionally for fbdev emulation, ..."?
I still don't know where you get that from - the armada fbdev code supports more than just 32bpp. Please explain.
On Wed, Mar 30, 2016 at 01:56:06PM +0100, Russell King - ARM Linux wrote:
On Wed, Mar 30, 2016 at 01:19:21PM +0200, Daniel Vetter wrote:
On Wed, Mar 30, 2016 at 1:09 PM, Russell King - ARM Linux linux@arm.linux.org.uk wrote:
On Wed, Mar 30, 2016 at 11:51:18AM +0200, Daniel Vetter wrote:
The fb helper private gamma_set/get functions are only required when the driver supports paletted 8bit mode with fbdev. Armada uses 32bpp unconditionally, so this is just dead code. It also doesn't do anything really. Let's just remove it.
This comment is misleading: Armada supports 16bpp formats as well as 32bpp, and the hardware does have 8bpp modes to, but I've chosen not to support the 8bpp modes.
This is purely about the fbdev emulation (and yeah need to clarify that), not about kms support in general. And these two gamma_set/get hooks are _only_ used by the fbdev emulation, and only needed if you do 8bit paletted mode. Ok if I change the commit message to "Armada used 32bpp unconditionally for fbdev emulation, ..."?
I still don't know where you get that from - the armada fbdev code supports more than just 32bpp. Please explain.
Ok, I lost myself in the code and missed the cmdline parsing of fbdev depth. So seems indeed possible to create a 8bit fbdev on armada (since armada_framebuffer_create won't reject it, and I didn't see anything else). I'll drop this patch. Thanks for the feedback. -Daniel
Only really needed for fbdev emulation at 8bpp. And bochs doesn't do that. And either way bochs only does 32bit rgb, so this is all pretty much wasted dead code.
The only consideration is that we need to not set up any gamma size either.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/bochs/bochs_fbdev.c | 15 --------------- drivers/gpu/drm/bochs/bochs_kms.c | 7 ------- 2 files changed, 22 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_fbdev.c b/drivers/gpu/drm/bochs/bochs_fbdev.c index 7520bf81fc25..369f11f10c72 100644 --- a/drivers/gpu/drm/bochs/bochs_fbdev.c +++ b/drivers/gpu/drm/bochs/bochs_fbdev.c @@ -162,22 +162,7 @@ static int bochs_fbdev_destroy(struct bochs_device *bochs) return 0; }
-void bochs_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green, - u16 blue, int regno) -{ -} - -void bochs_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, int regno) -{ - *red = regno; - *green = regno; - *blue = regno; -} - static const struct drm_fb_helper_funcs bochs_fb_helper_funcs = { - .gamma_set = bochs_fb_gamma_set, - .gamma_get = bochs_fb_gamma_get, .fb_probe = bochsfb_create, };
diff --git a/drivers/gpu/drm/bochs/bochs_kms.c b/drivers/gpu/drm/bochs/bochs_kms.c index 96926f09e0c9..89adfd916a7c 100644 --- a/drivers/gpu/drm/bochs/bochs_kms.c +++ b/drivers/gpu/drm/bochs/bochs_kms.c @@ -93,11 +93,6 @@ static void bochs_crtc_commit(struct drm_crtc *crtc) { }
-static void bochs_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, - u16 *blue, uint32_t start, uint32_t size) -{ -} - static int bochs_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, @@ -120,7 +115,6 @@ static int bochs_crtc_page_flip(struct drm_crtc *crtc,
/* These provide the minimum set of functions required to handle a CRTC */ static const struct drm_crtc_funcs bochs_crtc_funcs = { - .gamma_set = bochs_crtc_gamma_set, .set_config = drm_crtc_helper_set_config, .destroy = drm_crtc_cleanup, .page_flip = bochs_crtc_page_flip, @@ -140,7 +134,6 @@ static void bochs_crtc_init(struct drm_device *dev) struct drm_crtc *crtc = &bochs->crtc;
drm_crtc_init(dev, crtc, &bochs_crtc_funcs); - drm_mode_crtc_set_gamma_size(crtc, 256); drm_crtc_helper_add(crtc, &bochs_helper_funcs); }
On Mi, 2016-03-30 at 11:51 +0200, Daniel Vetter wrote:
Only really needed for fbdev emulation at 8bpp. And bochs doesn't do that. And either way bochs only does 32bit rgb, so this is all pretty much wasted dead code.
Pointless leftover when I copyed things over from cirrus.
Reviewed-by: Gerd Hoffmann kraxel@redhat.com
cheers, Gerd
The core does this for us already.
Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/cirrus/cirrus_mode.c | 3 --- 1 file changed, 3 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c index d3d8d7bfcc57..0b1a411cb89e 100644 --- a/drivers/gpu/drm/cirrus/cirrus_mode.c +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c @@ -331,9 +331,6 @@ static void cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc); int i;
- if (size != CIRRUS_LUT_SIZE) - return; - for (i = 0; i < CIRRUS_LUT_SIZE; i++) { cirrus_crtc->lut_r[i] = red[i]; cirrus_crtc->lut_g[i] = green[i];
Op 30-03-16 om 11:51 schreef Daniel Vetter:
The core does this for us already.
Unfortunately some other drivers do the same. :(
various variations of
end = min_t(start + size, 256);
I'll kill the rest off when killing start parameter and returning int.
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Again the fbdev emulation gamma_set/get functions are only needed for drivers that try to also use 8bpp paletted mode. Which msm doesn't, so this is dead code. Let's rip it out.
Cc: Rob Clark robdclark@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/msm/msm_fbdev.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c index d9759bf3482e..1a061e3e8b9e 100644 --- a/drivers/gpu/drm/msm/msm_fbdev.c +++ b/drivers/gpu/drm/msm/msm_fbdev.c @@ -184,21 +184,7 @@ fail: return ret; }
-static void msm_crtc_fb_gamma_set(struct drm_crtc *crtc, - u16 red, u16 green, u16 blue, int regno) -{ - DBG("fbdev: set gamma"); -} - -static void msm_crtc_fb_gamma_get(struct drm_crtc *crtc, - u16 *red, u16 *green, u16 *blue, int regno) -{ - DBG("fbdev: get gamma"); -} - static const struct drm_fb_helper_funcs msm_fb_helper_funcs = { - .gamma_set = msm_crtc_fb_gamma_set, - .gamma_get = msm_crtc_fb_gamma_get, .fb_probe = msm_fbdev_create, };
Op 30-03-16 om 11:51 schreef Daniel Vetter:
Again the fbdev emulation gamma_set/get functions are only needed for drivers that try to also use 8bpp paletted mode. Which msm doesn't, so this is dead code. Let's rip it out.
Cc: Rob Clark robdclark@gmail.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
No need to confuse userspace like this.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/virtio/virtgpu_display.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index 4854dac87e24..12b72e29678a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -38,13 +38,6 @@ #define XRES_MAX 8192 #define YRES_MAX 8192
-static void virtio_gpu_crtc_gamma_set(struct drm_crtc *crtc, - u16 *red, u16 *green, u16 *blue, - uint32_t start, uint32_t size) -{ - /* TODO */ -} - static void virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev, struct virtio_gpu_output *output) @@ -173,7 +166,6 @@ static int virtio_gpu_page_flip(struct drm_crtc *crtc, static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = { .cursor_set2 = virtio_gpu_crtc_cursor_set, .cursor_move = virtio_gpu_crtc_cursor_move, - .gamma_set = virtio_gpu_crtc_gamma_set, .set_config = drm_atomic_helper_set_config, .destroy = drm_crtc_cleanup,
@@ -416,7 +408,6 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index) return PTR_ERR(plane); drm_crtc_init_with_planes(dev, crtc, plane, NULL, &virtio_gpu_crtc_funcs, NULL); - drm_mode_crtc_set_gamma_size(crtc, 256); drm_crtc_helper_add(crtc, &virtio_gpu_crtc_helper_funcs); plane->crtc = crtc;
On Mi, 2016-03-30 at 11:51 +0200, Daniel Vetter wrote:
No need to confuse userspace like this.
Reviewed-by: Gerd Hoffmann kraxel@redhat.com
On Fri, Apr 01, 2016 at 10:38:09AM +0200, Gerd Hoffmann wrote:
On Mi, 2016-03-30 at 11:51 +0200, Daniel Vetter wrote:
No need to confuse userspace like this.
Reviewed-by: Gerd Hoffmann kraxel@redhat.com
This and the bochs one applied to drm-misc, thanks for your review. -Daniel
On 30 March 2016 at 10:51, Daniel Vetter daniel.vetter@ffwll.ch wrote:
No need to confuse userspace like this.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/virtio/virtgpu_display.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index 4854dac87e24..12b72e29678a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -38,13 +38,6 @@ #define XRES_MAX 8192 #define YRES_MAX 8192
-static void virtio_gpu_crtc_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green, u16 *blue,uint32_t start, uint32_t size)-{
/* TODO */-}
static void virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev, struct virtio_gpu_output *output) @@ -173,7 +166,6 @@ static int virtio_gpu_page_flip(struct drm_crtc *crtc, static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = { .cursor_set2 = virtio_gpu_crtc_cursor_set, .cursor_move = virtio_gpu_crtc_cursor_move,
.gamma_set = virtio_gpu_crtc_gamma_set, .set_config = drm_atomic_helper_set_config, .destroy = drm_crtc_cleanup,@@ -416,7 +408,6 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index) return PTR_ERR(plane); drm_crtc_init_with_planes(dev, crtc, plane, NULL, &virtio_gpu_crtc_funcs, NULL);
drm_mode_crtc_set_gamma_size(crtc, 256); drm_crtc_helper_add(crtc, &virtio_gpu_crtc_helper_funcs); plane->crtc = crtc;
Out of curiosity:
Coccinelle should be able to handle/generate such patches, shouldn't it ? I believe in the past people used it for similar refactoring/cleanups, yet not (m)any of them [the cocci files] got checked in the kernel tree.
Thinking about future drivers derived from outdated sources - do you think it's a good/bad idea to check/run them along side the existing ones ?
-Emil
On Tue, 12 Apr 2016, Emil Velikov wrote:
On 30 March 2016 at 10:51, Daniel Vetter daniel.vetter@ffwll.ch wrote:
No need to confuse userspace like this.
Cc: Gerd Hoffmann kraxel@redhat.com Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/virtio/virtgpu_display.c | 9 --------- 1 file changed, 9 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c b/drivers/gpu/drm/virtio/virtgpu_display.c index 4854dac87e24..12b72e29678a 100644 --- a/drivers/gpu/drm/virtio/virtgpu_display.c +++ b/drivers/gpu/drm/virtio/virtgpu_display.c @@ -38,13 +38,6 @@ #define XRES_MAX 8192 #define YRES_MAX 8192
-static void virtio_gpu_crtc_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green, u16 *blue,uint32_t start, uint32_t size)-{
/* TODO */-}
static void virtio_gpu_hide_cursor(struct virtio_gpu_device *vgdev, struct virtio_gpu_output *output) @@ -173,7 +166,6 @@ static int virtio_gpu_page_flip(struct drm_crtc *crtc, static const struct drm_crtc_funcs virtio_gpu_crtc_funcs = { .cursor_set2 = virtio_gpu_crtc_cursor_set, .cursor_move = virtio_gpu_crtc_cursor_move,
.gamma_set = virtio_gpu_crtc_gamma_set, .set_config = drm_atomic_helper_set_config, .destroy = drm_crtc_cleanup,@@ -416,7 +408,6 @@ static int vgdev_output_init(struct virtio_gpu_device *vgdev, int index) return PTR_ERR(plane); drm_crtc_init_with_planes(dev, crtc, plane, NULL, &virtio_gpu_crtc_funcs, NULL);
drm_mode_crtc_set_gamma_size(crtc, 256); drm_crtc_helper_add(crtc, &virtio_gpu_crtc_helper_funcs); plane->crtc = crtc;Out of curiosity:
Coccinelle should be able to handle/generate such patches, shouldn't it ? I believe in the past people used it for similar refactoring/cleanups, yet not (m)any of them [the cocci files] got checked in the kernel tree.
Thinking about future drivers derived from outdated sources - do you think it's a good/bad idea to check/run them along side the existing ones ?
The issue is that there is no point to put an empty function in a structure?
It would be a bit subtle for Coccinelle, because it requires also knowing that one is allowed to leave that particular field empty.
julia
imx doesn't have any functions for setting the gamma table, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Cc: Russell King rmk+kernel@arm.linux.org.uk Cc: Philipp Zabel p.zabel@pengutronix.de Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/imx/imx-drm-core.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 9876e0f0c3e1..21a52937f9cd 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -351,10 +351,6 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc,
*new_crtc = imx_drm_crtc;
- ret = drm_mode_crtc_set_gamma_size(imx_drm_crtc->crtc, 256); - if (ret) - goto err_register; - drm_crtc_helper_add(crtc, imx_drm_crtc->imx_drm_helper_funcs.crtc_helper_funcs);
Hi Daniel,
Am Mittwoch, den 30.03.2016, 11:51 +0200 schrieb Daniel Vetter:
imx doesn't have any functions for setting the gamma table, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Thank you for the patch. Now the ret variable and the err_register label are both unused, so I have applied it with the following fixup:
diff --git a/drivers/gpu/drm/imx/imx-drm-core.c b/drivers/gpu/drm/imx/imx-drm-core.c index 21a5293..e26dcde 100644 --- a/drivers/gpu/drm/imx/imx-drm-core.c +++ b/drivers/gpu/drm/imx/imx-drm-core.c @@ -326,7 +326,6 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, { struct imx_drm_device *imxdrm = drm->dev_private; struct imx_drm_crtc *imx_drm_crtc; - int ret;
/* * The vblank arrays are dimensioned by MAX_CRTC - we can't @@ -358,11 +357,6 @@ int imx_drm_add_crtc(struct drm_device *drm, struct drm_crtc *crtc, imx_drm_crtc->imx_drm_helper_funcs.crtc_funcs, NULL);
return 0; - -err_register: - imxdrm->crtc[--imxdrm->pipes] = NULL; - kfree(imx_drm_crtc); - return ret; } EXPORT_SYMBOL_GPL(imx_drm_add_crtc);
regards Philipp
qxl doesn't have any functions for setting the gamma table, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Cc: Dave Airlie airlied@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/qxl/qxl_display.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 030409a3ee4e..21c70952402d 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -729,7 +729,6 @@ static int qdev_crtc_init(struct drm_device *dev, int crtc_id)
drm_crtc_init(dev, &qxl_crtc->base, &qxl_crtc_funcs); qxl_crtc->index = crtc_id; - drm_mode_crtc_set_gamma_size(&qxl_crtc->base, 256); drm_crtc_helper_add(&qxl_crtc->base, &qxl_crtc_helper_funcs); return 0; }
Op 30-03-16 om 11:51 schreef Daniel Vetter:
qxl doesn't have any functions for setting the gamma table, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
On Tue, May 31, 2016 at 03:17:57PM +0200, Maarten Lankhorst wrote:
Op 30-03-16 om 11:51 schreef Daniel Vetter:
qxl doesn't have any functions for setting the gamma table, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Reviewed-by: Maarten Lankhorst maarten.lankhorst@linux.intel.com
Applied the other patches you've reviewed, too. Thanks for the reviews. -Daniel
tegra doesn't have any functions to set gamma tables, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/tegra/dc.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index fb2b4b0271a2..3b85a31b625d 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -1722,7 +1722,6 @@ static int tegra_dc_init(struct host1x_client *client) if (err < 0) goto cleanup;
- drm_mode_crtc_set_gamma_size(&dc->base, 256); drm_crtc_helper_add(&dc->base, &tegra_crtc_helper_funcs);
/*
On Wed, Mar 30, 2016 at 11:51:25AM +0200, Daniel Vetter wrote:
tegra doesn't have any functions to set gamma tables, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/tegra/dc.c | 1 - 1 file changed, 1 deletion(-)
Do you want this to go through the Tegra tree or do you have follow-up work that depends on this?
Thierry
On Wed, Mar 30, 2016 at 02:39:22PM +0200, Thierry Reding wrote:
On Wed, Mar 30, 2016 at 11:51:25AM +0200, Daniel Vetter wrote:
tegra doesn't have any functions to set gamma tables, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/tegra/dc.c | 1 - 1 file changed, 1 deletion(-)
Do you want this to go through the Tegra tree or do you have follow-up work that depends on this?
Driver trees is fine, pls just confirm when you do so (to make sure I don't accidentally merge it too). -Daniel
On Wed, Mar 30, 2016 at 11:51:25AM +0200, Daniel Vetter wrote:
tegra doesn't have any functions to set gamma tables, so this is completely defunct.
Not nice to lie to userspace, so let's stop!
Cc: Thierry Reding treding@nvidia.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com
drivers/gpu/drm/tegra/dc.c | 1 - 1 file changed, 1 deletion(-)
Applied, thanks.
Thierry
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
Emil Velikov -
Gerd Hoffmann -
Julia Lawall -
Maarten Lankhorst -
Philipp Zabel -
Russell King - ARM Linux -
Thierry Reding -
Thierry Reding