[Nouveau] [Bug 101573] GP107 crash with no HDMI connected on 4.12.rc6
https://bugs.freedesktop.org/show_bug.cgi?id=101573 --- Comment #2 from harry_x --- Versions: ArchLinux Kernel 4.12.rc6 xf86-video-nouveau 1.0.15-1 xf86-video-intel 1:2.99.917+777+g6babcf15-1 mesa 17.1.3-1 All are latest archlinux packages. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 101573] GP107 crash with no HDMI connected on 4.12.rc6
https://bugs.freedesktop.org/show_bug.cgi?id=101573 --- Comment #1 from harry_x --- Created attachment 132209 --> https://bugs.freedesktop.org/attachment.cgi?id=132209&action=edit dmesg output -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 101573] New: GP107 crash with no HDMI connected on 4.12.rc6
https://bugs.freedesktop.org/show_bug.cgi?id=101573 Bug ID: 101573 Summary: GP107 crash with no HDMI connected on 4.12.rc6 Product: xorg Version: unspecified Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Driver/nouveau Assignee: nouveau@lists.freedesktop.org Reporter: harr...@babylon5.cz QA Contact: xorg-t...@lists.x.org When starting the machine on Dell Inspiron 7000 (Kabylake, GTX 1050 Ti) with HDMI monitor connected (HDMI output is provided by NVIDIA card, eDP is connected to internal), everything seems to work (using reverse prime that is automatically setup). There is lot of tearing, but it works. But when starting the machine without HDMI output connected (so the NVIDIA card has no connected output), it fails with timeouts: čen 23 22:11:03 blackgate kernel: nouveau :01:00.0: DRM: resuming object tree... čen 23 22:11:03 blackgate kernel: nouveau :01:00.0: bus: MMIO read of FAULT at 409800 [ TIMEOUT ] čen 23 22:11:05 blackgate kernel: nouveau :01:00.0: timeout čen 23 22:11:05 blackgate kernel: [ cut here ] čen 23 22:11:05 blackgate kernel: WARNING: CPU: 6 PID: 138 at drivers/gpu/drm/nouveau/nvkm/engine/gr/gf100.c:1501 gf100_gr_init_ctxctl+0x81f/0x9a0 [nouveau] čen 23 22:11:05 blackgate kernel: Modules linked in: xt_nat veth xfs ipt_MASQUERADE nf_nat_masquerade_ipv4 nf_conntrack_netlink nfnetlink xfrm_user xfrm_algo xt_addrtype br_netfilter bridge stp llc tun dm_thin_pool dm_persistent_data dm_bi o_prison dm_bufio loop ecryptfs cbc encrypted_keys trusted ctr ccm cmac rfcomm pci_stub vboxpci(O) xt_tcpudp iptable_filter iptable_nat nf_conntrack_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4 nf_defrag_ipv6 nf_nat_ipv4 xt_conntrack nf_nat nf_co nntrack ip6table_filter ip6_tables libcrc32c crc32c_generic bnep joydev mousedev uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 hid_multitouch videobuf2_core videodev btusb btrtl media fuse snd_hda_codec_realtek snd_hda_codec_g eneric arc4 iTCO_wdt iTCO_vendor_support nls_iso8859_1 nls_cp437 vfat fat i2c_designware_platform iwlmvm i2c_designware_core čen 23 22:11:05 blackgate kernel: dell_wmi mac80211 dell_laptop dell_smbios dcdbas intel_rapl dell_smm_hwmon x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm iwlwifi irqbypass crct10dif_pclmul snd_hda_intel cfg80211 r8168(O) g hash_clmulni_intel snd_hda_codec intel_cstate intel_rapl_perf snd_hda_core psmouse snd_hwdep hci_uart input_leds snd_pcm btbcm idma64 btqca mei_me snd_timer btintel i2c_hid processor_thermal_device snd i2c_i801 mei soundcore intel_lpss_pci intel_pch_thermal bluetooth shpchp intel_soc_dts_iosf thermal hid battery int3403_thermal ecdh_generic tpm_crb rfkill intel_lpss_acpi intel_lpss int3402_thermal int340x_thermal_zone evdev intel_hid mac_hid int3400_thermal acpi_als sparse_ keymap tpm_tis acpi_thermal_rel kfifo_buf ac tpm_tis_core industrialio acpi_pad tpm sch_fq_codel msr vboxnetadp(O) čen 23 22:11:05 blackgate kernel: vboxnetflt(O) vboxdrv(O) i2c_dev sg ip_tables x_tables sd_mod ext4 crc16 jbd2 fscrypto mbcache dm_mod dax serio_raw atkbd libps2 crc32_pclmul crc32c_intel aesni_intel aes_x86_64 crypto_simd cryptd glue_he lper ahci libahci nouveau(-) libata led_class mxm_wmi scsi_mod xhci_pci ttm i8042 serio wmi i915 video button intel_gtt i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm xhci_hcd usbcore usb_common nvme nvme_cor e čen 23 22:11:05 blackgate kernel: CPU: 6 PID: 138 Comm: kworker/6:2 Tainted: G O4.12.0-rc6-mainline #1 čen 23 22:11:05 blackgate kernel: Hardware name: Dell Inc. Inspiron 15 7000 Gaming/065C71, BIOS 01.00.05 03/01/2017 čen 23 22:11:05 blackgate kernel: Workqueue: pm pm_runtime_work čen 23 22:11:05 blackgate kernel: task: 88085a909d80 task.stack: c90003abc000 čen 23 22:11:05 blackgate kernel: RIP: 0010:gf100_gr_init_ctxctl+0x81f/0x9a0 [nouveau] čen 23 22:11:05 blackgate kernel: RSP: 0018:c90003abfa08 EFLAGS: 00010286 čen 23 22:11:05 blackgate kernel: RAX: 001d RBX: 88085aa2f830 RCX: čen 23 22:11:05 blackgate kernel: RDX: RSI: 88087f58dcc8 RDI: 88087f58dcc8 čen 23 22:11:05 blackgate kernel: RBP: c90003abfa38 R08: 03e2 R09: 0004 čen 23 22:11:05 blackgate kernel: R10: c90003abf8a8 R11: 0001 R12: 880851cd čen 23 22:11:05 blackgate kernel: R13: 77363ba0 R14: 8808526ddba0 R15: 001cebd02520 čen 23 22:11:05 blackgate kernel: FS: () GS:88087f58() knlGS: čen 23 22:11:05 blackgate kernel: CS: 0010 DS: ES: CR0: 80050033 čen 23 22:11:05 blackgate kernel: CR2: 0232146c CR3: 01a09000 CR4: 003406e0 čen 23 22:11:05 blackgate kernel: Call Trace: čen 23 22:11:05 blackgate kernel: gp100_
[Nouveau] [Bug 100228] [NV137] bus: MMIO read of 00000000 FAULT at 409800 [ TIMEOUT ]
https://bugs.freedesktop.org/show_bug.cgi?id=100228 Rhys Kidd changed: What|Removed |Added Summary|[NV137] unknown chipset |[NV137] bus: MMIO read of |(137000a1) | FAULT at 409800 [ ||TIMEOUT ] -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage
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, if it helps you to ease the drm framework
update then I am agree to "acknowledge it" asap, else if you are not in
a hurry, I would prefer a better and definitive patch handling 8-bit
CLUT properly and I am ok to help or/and to do it : )
Extra questions:
- any plan to update modetest with the DRM_FORMAT_C8 support + gamma
get/set?
- do you have a simple way to test clut with fbdev, last year we where
using an old version of the SDL but I am still looking for a small piece
of code to do it (else I will do it myself but C8 on fbdev is not really
a priority ;-)
best regards,
Philippe
> Just remove the dead code.
>
> Signed-off-by: Peter Rosin
> ---
> drivers/gpu/drm/stm/ltdc.c | 12
> drivers/gpu/drm/stm/ltdc.h | 1 -
> 2 files changed, 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index 1b9483d..87829b9 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
>* DRM_CRTC
>*/
>
> -static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
> -{
> - struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> - unsigned int i, lay;
> -
> - for (lay = 0; lay < ldev->caps.nb_layers; lay++)
> - for (i = 0; i < 256; i++)
> - reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
> - ldev->clut[i]);
> -}
> -
> static void ltdc_crtc_enable(struct drm_crtc *crtc)
> {
> struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
> }
>
> static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
> - .load_lut = ltdc_crtc_load_lut,
> .enable = ltdc_crtc_enable,
> .disable = ltdc_crtc_disable,
> .mode_set_nofb = ltdc_crtc_mode_set_nofb,
> diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> index d7a9c73..620ca55 100644
> --- a/drivers/gpu/drm/stm/ltdc.h
> +++ b/drivers/gpu/drm/stm/ltdc.h
> @@ -27,7 +27,6 @@ struct ltdc_device {
> struct drm_panel *panel;
> struct mutex err_lock; /* protecting error_status */
> struct ltdc_caps caps;
> - u32 clut[256]; /* color look up table */
> u32 error_status;
> u32 irq_status;
> };
>
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 11/14] drm: nouveau: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 -
drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 ---
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 --
drivers/gpu/drm/nouveau/nv50_display.c | 40 +++--
4 files changed, 22 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b4..8f689f1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -764,13 +764,18 @@ nv_crtc_gamma_load(struct drm_crtc *crtc)
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
struct drm_device *dev = nv_crtc->base.dev;
struct rgb { uint8_t r, g, b; } __attribute__((packed)) *rgbs;
+ u16 *r, *g, *b;
int i;
rgbs = (struct rgb
*)nv04_display(dev)->mode_reg.crtc_reg[nv_crtc->index].DAC;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++) {
- rgbs[i].r = nv_crtc->lut.r[i] >> 8;
- rgbs[i].g = nv_crtc->lut.g[i] >> 8;
- rgbs[i].b = nv_crtc->lut.b[i] >> 8;
+ rgbs[i].r = *r++ >> 8;
+ rgbs[i].g = *g++ >> 8;
+ rgbs[i].b = *b++ >> 8;
}
nouveau_hw_load_state_palette(dev, nv_crtc->index,
&nv04_display(dev)->mode_reg);
@@ -792,13 +797,6 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g,
u16 *b,
struct drm_modeset_acquire_ctx *ctx)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- nv_crtc->lut.r[i] = r[i];
- nv_crtc->lut.g[i] = g[i];
- nv_crtc->lut.b[i] = b[i];
- }
/* We need to know the depth before we upload, but it's possible to
* get called before a framebuffer is bound. If this is the case,
@@ -1095,7 +1093,6 @@ static const struct drm_crtc_helper_funcs
nv04_crtc_helper_funcs = {
.mode_set = nv_crtc_mode_set,
.mode_set_base = nv04_crtc_mode_set_base,
.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
- .load_lut = nv_crtc_gamma_load,
.disable = nv_crtc_disable,
};
@@ -1103,17 +1100,12 @@ int
nv04_crtc_create(struct drm_device *dev, int crtc_num)
{
struct nouveau_crtc *nv_crtc;
- int ret, i;
+ int ret;
nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
if (!nv_crtc)
return -ENOMEM;
- for (i = 0; i < 256; i++) {
- nv_crtc->lut.r[i] = i << 8;
- nv_crtc->lut.g[i] = i << 8;
- nv_crtc->lut.b[i] = i << 8;
- }
nv_crtc->lut.depth = 0;
nv_crtc->index = crtc_num;
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h
b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index 050fcf3..b7a18fb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
+++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
@@ -61,9 +61,6 @@ struct nouveau_crtc {
struct {
struct nouveau_bo *nvbo;
- uint16_t r[256];
- uint16_t g[256];
- uint16_t b[256];
int depth;
} lut;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 2665a07..f770784 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -278,26 +278,6 @@ nouveau_fbcon_accel_init(struct drm_device *dev)
info->fbops = &nouveau_fbcon_ops;
}
-static void nouveau_fbcon_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
- nv_crtc->lut.r[regno] = red;
- nv_crtc->lut.g[regno] = green;
- nv_crtc->lut.b[regno] = blue;
-}
-
-static void nouveau_fbcon_gamma_get(struct drm_crtc *crtc, u16 *red, u16
*green,
- u16 *blue, int regno)
-{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
- *red = nv_crtc->lut.r[regno];
- *green = nv_crtc->lut.g[regno];
- *blue = nv_crtc->lut.b[regno];
-}
-
static void
nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon)
{
@@ -467,8 +447,6 @@ void nouveau_fbcon_gpu_lockup(struct fb_info *info)
}
static const struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
- .gamma_set = nouveau_fbcon_gamma_set,
- .gamma_get = nouveau_fbcon_gamma_get,
.fb_probe = nouveau_fbcon_create,
};
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
b/drivers/gpu/drm/nouveau/nv50_display.c
index e3132a2
[Nouveau] [PATCH 11/11] drm: remove unused and redundant callbacks
Drivers no longer have any need for these callbacks, and there are no
users. Zap. Zap-zap-zzzap-p-pp-p.
Signed-off-by: Peter Rosin
---
include/drm/drm_fb_helper.h | 32
include/drm/drm_modeset_helper_vtables.h | 16
2 files changed, 48 deletions(-)
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 119e5e4..80d9853 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -85,38 +85,6 @@ struct drm_fb_helper_surface_size {
*/
struct drm_fb_helper_funcs {
/**
-* @gamma_set:
-*
-* Set the given gamma LUT register on the given CRTC.
-*
-* This callback is optional.
-*
-* FIXME:
-*
-* 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 exists because the fbdev hasn't yet been
-* refactored to use the core gamma table interfaces.
-*/
- void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno);
-
- /**
* @fb_probe:
*
* Driver callback to allocate and initialize the fbdev info structure.
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index 85984b2..0773db9 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -267,22 +267,6 @@ struct drm_crtc_helper_funcs {
enum mode_set_atomic);
/**
-* @load_lut:
-*
-* Load a LUT prepared with the &drm_fb_helper_funcs.gamma_set vfunc.
-*
-* This callback is optional and is only used by the fbdev emulation
-* helpers.
-*
-* FIXME:
-*
-* 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 (*load_lut)(struct drm_crtc *crtc);
-
- /**
* @disable:
*
* This callback should be used to disable the CRTC. With the atomic
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH 10/11] drm: stm: remove dead code and pointless local lut storage
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.
Just remove the dead code.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/stm/ltdc.c | 12
drivers/gpu/drm/stm/ltdc.h | 1 -
2 files changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 1b9483d..87829b9 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
* DRM_CRTC
*/
-static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
-{
- struct ltdc_device *ldev = crtc_to_ltdc(crtc);
- unsigned int i, lay;
-
- for (lay = 0; lay < ldev->caps.nb_layers; lay++)
- for (i = 0; i < 256; i++)
- reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
- ldev->clut[i]);
-}
-
static void ltdc_crtc_enable(struct drm_crtc *crtc)
{
struct ltdc_device *ldev = crtc_to_ltdc(crtc);
@@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
}
static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
- .load_lut = ltdc_crtc_load_lut,
.enable = ltdc_crtc_enable,
.disable = ltdc_crtc_disable,
.mode_set_nofb = ltdc_crtc_mode_set_nofb,
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index d7a9c73..620ca55 100644
--- a/drivers/gpu/drm/stm/ltdc.h
+++ b/drivers/gpu/drm/stm/ltdc.h
@@ -27,7 +27,6 @@ struct ltdc_device {
struct drm_panel *panel;
struct mutex err_lock; /* protecting error_status */
struct ltdc_caps caps;
- u32 clut[256]; /* color look up table */
u32 error_status;
u32 irq_status;
};
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 10/14] drm: mgag200: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ---
drivers/gpu/drm/mgag200/mgag200_fb.c | 2 --
drivers/gpu/drm/mgag200/mgag200_mode.c | 62 --
3 files changed, 15 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index c88b6ec..04f1dfb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -237,11 +237,6 @@ mgag200_bo(struct ttm_buffer_object *bo)
{
return container_of(bo, struct mgag200_bo, bo);
}
- /* mgag200_crtc.c */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, int regno);
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-u16 *blue, int regno);
/* mgag200_mode.c */
int mgag200_modeset_init(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c
b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5d3b1fa..5cf980a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -258,8 +258,6 @@ static int mga_fbdev_destroy(struct drm_device *dev,
}
static const struct drm_fb_helper_funcs mga_fb_helper_funcs = {
- .gamma_set = mga_crtc_fb_gamma_set,
- .gamma_get = mga_crtc_fb_gamma_get,
.fb_probe = mgag200fb_create,
};
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index f4b5358..5e9cd4c 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -27,15 +27,19 @@
static void mga_crtc_load_lut(struct drm_crtc *crtc)
{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct mga_device *mdev = dev->dev_private;
struct drm_framebuffer *fb = crtc->primary->fb;
+ u16 *r_ptr, *g_ptr, *b_ptr;
int i;
if (!crtc->enabled)
return;
+ r_ptr = crtc->gamma_store;
+ g_ptr = r_ptr + crtc->gamma_size;
+ b_ptr = g_ptr + crtc->gamma_size;
+
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
if (fb && fb->format->cpp[0] * 8 == 16) {
@@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
if (i > (MGAG200_LUT_SIZE >> 1)) {
r = b = 0;
} else {
- r = mga_crtc->lut_r[i << 1];
- b = mga_crtc->lut_b[i << 1];
+ r = *r_ptr++ >> 8;
+ b = *b_ptr++ >> 8;
+ r_ptr++;
+ b_ptr++;
}
} else {
- r = mga_crtc->lut_r[i];
- b = mga_crtc->lut_b[i];
+ r = *r_ptr++ >> 8;
+ b = *b_ptr++ >> 8;
}
/* VGA registers */
WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
}
return;
}
for (i = 0; i < MGAG200_LUT_SIZE; i++) {
/* VGA registers */
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8);
}
}
@@ -1399,14 +1405,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16
*red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- mga_crtc->lut_r[i] = red[i] >> 8;
- mga_crtc->lut_g[i] = green[i] >> 8;
- mga_crtc->lut_b[i] = blue[i] >> 8;
- }
mga_crtc_load_lut(crtc);
return 0;
@@ -1455,14 +1453,12 @@ static const struct drm_crtc_helper_funcs
mga_helper_funcs = {
.mode_set_base = mga_crtc_mode_set_base,
Re: [Nouveau] [PATCH 08/11] drm: nouveau: remove dead code and pointless local lut storage
On 2017-06-20 21:25, Peter Rosin wrote:
> The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
> no longer used. Remove the dead code and hook up the crtc .gamma_set
> to use the crtc gamma_store directly instead of duplicating that
> info locally.
[...]
> - for (i = 0; i < 256; i++) {
> - u16 r = nv_crtc->lut.r[i] >> 2;
> - u16 g = nv_crtc->lut.g[i] >> 2;
> - u16 b = nv_crtc->lut.b[i] >> 2;
> + r = crtc->gamma_store;
> + g = r + crtc->gamma_size;
> + b = g + crtc->gamma_size;
>
> + for (i = 0; i < 256; i++) {
> if (disp->disp->oclass < GF110_DISP) {
> - writew(r + 0x, lut + (i * 0x08) + 0);
> - writew(g + 0x, lut + (i * 0x08) + 2);
> - writew(b + 0x, lut + (i * 0x08) + 4);
> + writew((*r++ >> 2) + 0x, lut + (i * 0x08) + 0);
> + writew((*g++ >> 2) + 0x, lut + (i * 0x08) + 2);
> + writew((*b++ >> 2) + 0x, lut + (i * 0x08) + 4);
> } else {
> - writew(r + 0x6000, lut + (i * 0x20) + 0);
> - writew(g + 0x6000, lut + (i * 0x20) + 2);
> - writew(b + 0x6000, lut + (i * 0x20) + 4);
> + writew((*r++ >> 2) + 0x6000, lut + (i * 0x20) + 0);
> + writew((*g++ >> 2) + 0x6000, lut + (i * 0x20) + 2);
> + writew((*b++ >> 2) + 0x6000, lut + (i * 0x20) + 4);
> }
> }
> }
I forgot to mention this, but the above is very strange for
disp->disp->oclass >= GF110_DISP because 0x6000 interferes with
the 14 bits that appear to be the lut depth in the registers.
I suspect some other bit-shift should be used for that case?
Someone should probably consult a datasheet...
Cheers,
peda
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH 02/11] drm: amd: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 -
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 +++
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 +++
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c| 27 +++
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c| 27 +++
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 23 ---
7 files changed, 28 insertions(+), 128 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index c0d8c6f..7dc3780 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -312,31 +312,7 @@ static int amdgpu_fbdev_destroy(struct drm_device *dev,
struct amdgpu_fbdev *rfb
return 0;
}
-/** Sets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
- amdgpu_crtc->lut_r[regno] = red >> 6;
- amdgpu_crtc->lut_g[regno] = green >> 6;
- amdgpu_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16
*green,
- u16 *blue, int regno)
-{
- struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
- *red = amdgpu_crtc->lut_r[regno] << 6;
- *green = amdgpu_crtc->lut_g[regno] << 6;
- *blue = amdgpu_crtc->lut_b[regno] << 6;
-}
-
static const struct drm_fb_helper_funcs amdgpu_fb_helper_funcs = {
- .gamma_set = amdgpu_crtc_fb_gamma_set,
- .gamma_get = amdgpu_crtc_fb_gamma_get,
.fb_probe = amdgpufb_create,
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 43a9d3a..39f7eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -369,7 +369,6 @@ struct amdgpu_atom_ss {
struct amdgpu_crtc {
struct drm_crtc base;
int crtc_id;
- u16 lut_r[256], lut_g[256], lut_b[256];
bool enabled;
bool can_tile;
uint32_t crtc_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 3c62c45..8e8c028 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2264,6 +2264,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct amdgpu_device *adev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
u32 tmp;
@@ -2301,11 +2302,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc
*crtc)
WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x0007);
WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
- (amdgpu_crtc->lut_r[i] << 20) |
- (amdgpu_crtc->lut_g[i] << 10) |
- (amdgpu_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2621,15 +2625,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc
*crtc, u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
- int i;
-
- /* userspace palettes are always correct as is */
- for (i = 0; i < size; i++) {
- amdgpu_crtc->lut_r[i] = red[i] >> 6;
- amdgpu_crtc->lut_g[i] = green[i] >> 6;
- amdgpu_crtc->lut_b[i] = blue[i] >> 6;
- }
dce_v10_0_crtc_load_lut(crtc);
return 0;
@@ -2841,14 +2836,12 @@ static const struct drm_crtc_helper_funcs
dce_v10_0_crtc_helper_funcs = {
.mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
.prepare = dce_v10_0_crtc_prepare,
.commit = dce_v10_0_crtc_commit,
- .load_lut = dce_v10_0_crtc_load_lut,
.disable = dce_v10_0_crtc_disable,
};
static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int ind
[Nouveau] [PATCH 05/11] dmr: gma500: remove dead code and pointless local lut storage
The redundant fb helpers .gamma_set and .gamma_get are no longer
used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
It is a bit strange that the fb helper .load_lut was not hooked
up, so this change may well make the driver work for the C8 mode
from the fbdev interface. But that is untested.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/gma500/framebuffer.c | 22
drivers/gpu/drm/gma500/gma_display.c | 32 ++
drivers/gpu/drm/gma500/psb_intel_display.c | 7 +--
drivers/gpu/drm/gma500/psb_intel_drv.h | 1 -
4 files changed, 12 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index 7da70b6..2570c7f 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -479,26 +479,6 @@ static struct drm_framebuffer *psb_user_framebuffer_create
return psb_framebuffer_create(dev, cmd, r);
}
-static void psbfb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
- gma_crtc->lut_r[regno] = red >> 8;
- gma_crtc->lut_g[regno] = green >> 8;
- gma_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void psbfb_gamma_get(struct drm_crtc *crtc, u16 *red,
- u16 *green, u16 *blue, int regno)
-{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
- *red = gma_crtc->lut_r[regno] << 8;
- *green = gma_crtc->lut_g[regno] << 8;
- *blue = gma_crtc->lut_b[regno] << 8;
-}
-
static int psbfb_probe(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
@@ -525,8 +505,6 @@ static int psbfb_probe(struct drm_fb_helper *helper,
}
static const struct drm_fb_helper_funcs psb_fb_helper_funcs = {
- .gamma_set = psbfb_gamma_set,
- .gamma_get = psbfb_gamma_get,
.fb_probe = psbfb_probe,
};
diff --git a/drivers/gpu/drm/gma500/gma_display.c
b/drivers/gpu/drm/gma500/gma_display.c
index e7fd356..f3c48a2 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -144,33 +144,32 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
const struct psb_offset *map = &dev_priv->regmap[gma_crtc->pipe];
int palreg = map->palette;
+ u16 *r, *g, *b;
int i;
/* The clocks have to be on to load the palette. */
if (!crtc->enabled)
return;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
if (gma_power_begin(dev, false)) {
for (i = 0; i < 256; i++) {
REG_WRITE(palreg + 4 * i,
- ((gma_crtc->lut_r[i] +
- gma_crtc->lut_adj[i]) << 16) |
- ((gma_crtc->lut_g[i] +
- gma_crtc->lut_adj[i]) << 8) |
- (gma_crtc->lut_b[i] +
- gma_crtc->lut_adj[i]));
+ (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+ (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+ ((*b++ >> 8) + gma_crtc->lut_adj[i]));
}
gma_power_end(dev);
} else {
for (i = 0; i < 256; i++) {
/* FIXME: Why pipe[0] and not pipe[..._crtc->pipe]? */
dev_priv->regs.pipe[0].palette[i] =
- ((gma_crtc->lut_r[i] +
- gma_crtc->lut_adj[i]) << 16) |
- ((gma_crtc->lut_g[i] +
- gma_crtc->lut_adj[i]) << 8) |
- (gma_crtc->lut_b[i] +
- gma_crtc->lut_adj[i]);
+ (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+ (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+ ((*b++ >> 8) + gma_crtc->lut_adj[i]);
}
}
@@ -180,15 +179,6 @@ int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
u16 *green, u16 *blue,
u32 size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- gma_crtc->lut_r[i] = red[i] >> 8;
- gma_crtc->lut_g[i] = green[i] >> 8;
- gma_crtc->lut_b[i] = blue[i] >> 8;
- }
-
gma_crtc_load_lut(crtc);
return 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_d
[Nouveau] [PATCH v2 00/14] improve the fb_setcmap helper
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. I have obviously not tested all of this with more than a compile, but patches 1 and 3 are enough to make the atmel-hlcdc driver do what I need (when patched to support CLUT modes). The rest is just lots of removals and cleanup made possible by the improved core. Please test, I would not be 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_VISUAL_TRUECOLOR. - Add a comment about colliding bitfields in the nouveau driver. - Remove gamma_set/gamma_get declarations from the radeon driver (the definitions were removed in v1). Cheers, peda Peter Rosin (14): drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap drm/fb-helper: remove drm_fb_helper_save_lut_atomic drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set drm: amd: remove dead code and pointless local lut storage drm: armada: remove dead empty functions drm: ast: remove dead code and pointless local lut storage drm: cirrus: remove dead code and pointless local lut storage drm: gma500: remove dead code and pointless local lut storage drm: i915: remove dead code and pointless local lut storage drm: mgag200: remove dead code and pointless local lut storage drm: nouveau: remove dead code and pointless local lut storage drm: radeon: remove dead code and pointless local lut storage drm: stm: remove dead code and pointless local lut storage drm: remove unused and redundant callbacks drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 - drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 ++--- drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 ++--- drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 27 ++--- drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 27 ++--- drivers/gpu/drm/amd/amdgpu/dce_virtual.c| 23 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/ast/ast_drv.h | 1 - drivers/gpu/drm/ast/ast_fb.c| 20 drivers/gpu/drm/ast/ast_mode.c | 26 + drivers/gpu/drm/cirrus/cirrus_drv.h | 8 -- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 - drivers/gpu/drm/cirrus/cirrus_mode.c| 71 +++- drivers/gpu/drm/drm_fb_helper.c | 164 +--- drivers/gpu/drm/gma500/framebuffer.c| 22 drivers/gpu/drm/gma500/gma_display.c| 32 ++ drivers/gpu/drm/gma500/psb_intel_display.c | 7 +- drivers/gpu/drm/gma500/psb_intel_drv.h | 1 - drivers/gpu/drm/i915/intel_drv.h| 1 - drivers/gpu/drm/i915/intel_fbdev.c | 31 -- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 - drivers/gpu/drm/mgag200/mgag200_fb.c| 2 - drivers/gpu/drm/mgag200/mgag200_mode.c | 62 +++ drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 ++--- drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 - drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 drivers/gpu/drm/nouveau/nv50_display.c | 40 +++ drivers/gpu/drm/radeon/atombios_crtc.c | 1 - drivers/gpu/drm/radeon/radeon_connectors.c | 7 +- drivers/gpu/drm/radeon/radeon_display.c | 71 +--- drivers/gpu/drm/radeon/radeon_fb.c | 2 - drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 - drivers/gpu/drm/radeon/radeon_mode.h| 4 - drivers/gpu/drm/stm/ltdc.c | 12 -- drivers/gpu/drm/stm/ltdc.h | 1 - include/drm/drm_fb_helper.h | 32 -- include/drm/drm_modeset_helper_vtables.h| 16 --- 40 files changed, 205 insertions(+), 658 deletions(-) -- 2.1.4 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage
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...). > > Regarding your patch below, if it helps you to ease the drm framework > update then I am agree to "acknowledge it" asap, else if you are not in > a hurry, I would prefer a better and definitive patch handling 8-bit > CLUT properly and I am ok to help or/and to do it : ) Hi! The thing is, without my series you will have to provide four callbacks. The crtc .gamma_set and the three redundant fb helpers .gamma_get, .gamma_set and .load_lut that pretty much does exactly what the crtc .gamma_set is doing. Well not .gamma_get, but... With my series, you only have to provide the crtc .gamma_set, which you have to provide anyway. and ...the core will handle everything that .gamma_get was used for... I.e., your work to provide CLUT support should start with drm support, which means the crtc .gamma_set, and then move on to the fbdev emulation. And I have just eliminated the second step for you, and as suger on top, you no longer have to convince the core drm maintainers that adding a lot of fbdev emulation code is needed. So, I think you actually want to wait for my series to land before adding CLUT support. > Extra questions: > - any plan to update modetest with the DRM_FORMAT_C8 support + gamma > get/set? I don't know that code base at all, but from the glimpse I got when browsing it, it seemed like it was pretty hardwired to non-palettized modes. I ended up having no need for it, see below... > - do you have a simple way to test clut with fbdev, last year we where > using an old version of the SDL but I am still looking for a small piece > of code to do it (else I will do it myself but C8 on fbdev is not really > a priority ;-) I'm doing pretty much the same thing, I have an application that requires an old SDL, and I'm using the programs/demos/demo.c program from the very old libggi as a second test app. But that's just because libggi is what I'm most familiar with, and it doesn't try to be "nice" and do things automatically, instead you have to manually insert helpers providing e.g. palette emulation if the application assumes a palettized mode and only truecolor modes are available from the HW. SDL tends to add those things for you, making it less easy to test thing, but I'm not an "SDL-guy", so there may very well exist some knobs I don't know about. Oh, you probably didn't see this: http://marc.info/?l=linux-kernel&m=149786920731175&w=4 It sports modeset-pal.c that sets the C8 mode, and does a 5 second palette animation, w/o using fbdev. I used it instead of digging further into modetest. Cheers, peda ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 07/14] drm: cirrus: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/cirrus/cirrus_drv.h | 8
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 -
drivers/gpu/drm/cirrus/cirrus_mode.c | 71 ---
3 files changed, 16 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 8690352..be2d7e48 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -96,7 +96,6 @@
struct cirrus_crtc {
struct drm_crtc base;
- u8 lut_r[256], lut_g[256], lut_b[256];
int last_dpms;
boolenabled;
};
@@ -180,13 +179,6 @@ cirrus_bo(struct ttm_buffer_object *bo)
#define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
#define DRM_FILE_PAGE_OFFSET (0x1ULL >> PAGE_SHIFT)
- /* cirrus_mode.c */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, int regno);
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-u16 *blue, int regno);
-
-
/* cirrus_main.c */
int cirrus_device_init(struct cirrus_device *cdev,
struct drm_device *ddev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 7fa58ee..1fedab0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -265,8 +265,6 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
}
static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
- .gamma_set = cirrus_crtc_fb_gamma_set,
- .gamma_get = cirrus_crtc_fb_gamma_get,
.fb_probe = cirrusfb_create,
};
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 53f6f0f..a4c4a46 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
* This file contains setup code for the CRTC.
*/
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
- struct drm_device *dev = crtc->dev;
- struct cirrus_device *cdev = dev->dev_private;
- int i;
-
- if (!crtc->enabled)
- return;
-
- for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
- /* VGA registers */
- WREG8(PALETTE_INDEX, i);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
- }
-}
-
/*
* The DRM core requires DPMS functions, but they make little sense in our
* case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+ struct drm_device *dev = crtc->dev;
+ struct cirrus_device *cdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
- for (i = 0; i < size; i++) {
- cirrus_crtc->lut_r[i] = red[i];
- cirrus_crtc->lut_g[i] = green[i];
- cirrus_crtc->lut_b[i] = blue[i];
+ if (!crtc->enabled)
+ return 0;
+
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
+ for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+ /* VGA registers */
+ WREG8(PALETTE_INDEX, i);
+ WREG8(PALETTE_DATA, *r++ >> 8);
+ WREG8(PALETTE_DATA, *g++ >> 8);
+ WREG8(PALETTE_DATA, *b++ >> 8);
}
- cirrus_crtc_load_lut(crtc);
return 0;
}
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs
cirrus_helper_funcs = {
.mode_set_base = cirrus_crtc_mode_set_base,
.prepare = cirrus_crtc_prepare,
.commit = cirrus_crtc_commit,
- .load_lut = cirrus_crtc_load_lut,
};
/* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
{
struct cirrus_device *cdev = dev->dev_private;
struct cirrus_crtc *cirrus_crtc;
- int i;
cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
(CIRRUSFB_CONN_LIMIT * sizeof(struct
drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
cde
Re: [Nouveau] [Intel-gfx] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage
On Thu, Jun 22, 2017 at 11:49:34AM +, 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...).
>
> Regarding your patch below, if it helps you to ease the drm framework
> update then I am agree to "acknowledge it" asap, else if you are not in
> a hurry, I would prefer a better and definitive patch handling 8-bit
> CLUT properly and I am ok to help or/and to do it : )
I'll take your ack, since your 8bit lut support will be massively simpler
with Peter's series here :-)
> Extra questions:
> - any plan to update modetest with the DRM_FORMAT_C8 support + gamma
> get/set?
We have gamma igts, well for the fancy new atomic color manager stuff.
Testing drivers with modetest is kinda not that awesome :-)
-Daniel
> - do you have a simple way to test clut with fbdev, last year we where
> using an old version of the SDL but I am still looking for a small piece
> of code to do it (else I will do it myself but C8 on fbdev is not really
> a priority ;-)
>
> best regards,
> Philippe
>
> > Just remove the dead code.
> >
> > Signed-off-by: Peter Rosin
> > ---
> > drivers/gpu/drm/stm/ltdc.c | 12
> > drivers/gpu/drm/stm/ltdc.h | 1 -
> > 2 files changed, 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> > index 1b9483d..87829b9 100644
> > --- a/drivers/gpu/drm/stm/ltdc.c
> > +++ b/drivers/gpu/drm/stm/ltdc.c
> > @@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
> >* DRM_CRTC
> >*/
> >
> > -static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
> > -{
> > - struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > - unsigned int i, lay;
> > -
> > - for (lay = 0; lay < ldev->caps.nb_layers; lay++)
> > - for (i = 0; i < 256; i++)
> > - reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
> > - ldev->clut[i]);
> > -}
> > -
> > static void ltdc_crtc_enable(struct drm_crtc *crtc)
> > {
> > struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> > @@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc
> > *crtc,
> > }
> >
> > static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
> > - .load_lut = ltdc_crtc_load_lut,
> > .enable = ltdc_crtc_enable,
> > .disable = ltdc_crtc_disable,
> > .mode_set_nofb = ltdc_crtc_mode_set_nofb,
> > diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
> > index d7a9c73..620ca55 100644
> > --- a/drivers/gpu/drm/stm/ltdc.h
> > +++ b/drivers/gpu/drm/stm/ltdc.h
> > @@ -27,7 +27,6 @@ struct ltdc_device {
> > struct drm_panel *panel;
> > struct mutex err_lock; /* protecting error_status */
> > struct ltdc_caps caps;
> > - u32 clut[256]; /* color look up table */
> > u32 error_status;
> > u32 irq_status;
> > };
> >
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH 00/11] improve the fb_setcmap helper
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 enough to make the atmel-hlcdc driver do what I need. The rest is just lots of removals and cleanup made possible by the improved core. Please test, I would not be surprised if I have fouled up some bit-manipulation somewhere in this mostly mechanical change... Cheers, peda Peter Rosin (11): drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set drm: amd: remove dead code and pointless local lut storage drm: ast: remove dead code and pointless local lut storage drm: cirrus: remove dead code and pointless local lut storage dmr: gma500: remove dead code and pointless local lut storage drm: i915: remove dead code and pointless local lut storage drm: mgag200: remove dead code and pointless local lut storage drm: nouveau: remove dead code and pointless local lut storage drm: radeon: remove dead code and pointless local lut storage drm: stm: remove dead code and pointless local lut storage drm: remove unused and redundant callbacks drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24 - drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h| 1 - drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 ++ drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 ++ drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 27 ++ drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 27 ++ drivers/gpu/drm/amd/amdgpu/dce_virtual.c| 23 - drivers/gpu/drm/ast/ast_drv.h | 1 - drivers/gpu/drm/ast/ast_fb.c| 20 - drivers/gpu/drm/ast/ast_mode.c | 26 ++ drivers/gpu/drm/cirrus/cirrus_drv.h | 8 -- drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 - drivers/gpu/drm/cirrus/cirrus_mode.c| 71 --- drivers/gpu/drm/drm_fb_helper.c | 131 +--- drivers/gpu/drm/gma500/framebuffer.c| 22 - drivers/gpu/drm/gma500/gma_display.c| 32 +++ drivers/gpu/drm/gma500/psb_intel_display.c | 7 +- drivers/gpu/drm/gma500/psb_intel_drv.h | 1 - drivers/gpu/drm/i915/intel_drv.h| 1 - drivers/gpu/drm/i915/intel_fbdev.c | 31 --- drivers/gpu/drm/mgag200/mgag200_drv.h | 5 -- drivers/gpu/drm/mgag200/mgag200_fb.c| 2 - drivers/gpu/drm/mgag200/mgag200_mode.c | 62 - drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 ++ drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 - drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 - drivers/gpu/drm/nouveau/nv50_display.c | 39 +++-- drivers/gpu/drm/radeon/atombios_crtc.c | 1 - drivers/gpu/drm/radeon/radeon_connectors.c | 7 +- drivers/gpu/drm/radeon/radeon_display.c | 71 ++- drivers/gpu/drm/radeon/radeon_fb.c | 2 - drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 - drivers/gpu/drm/stm/ltdc.c | 12 --- drivers/gpu/drm/stm/ltdc.h | 1 - include/drm/drm_fb_helper.h | 32 --- include/drm/drm_modeset_helper_vtables.h| 16 36 files changed, 171 insertions(+), 640 deletions(-) -- 2.1.4 ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
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 the gamma_store which should already be up to date with what
.gamma_get would return and is thus a nop. So, zap it.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/drm_fb_helper.c | 131
1 file changed, 40 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 574af01..cc2d55d 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct
drm_fb_helper *fb_helper,
}
EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
-static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct
drm_fb_helper *helper)
-{
- uint16_t *r_base, *g_base, *b_base;
- int i;
-
- if (helper->funcs->gamma_get == NULL)
- return;
-
- r_base = crtc->gamma_store;
- g_base = r_base + crtc->gamma_size;
- b_base = g_base + crtc->gamma_size;
-
- for (i = 0; i < crtc->gamma_size; i++)
- helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i],
&b_base[i], i);
-}
-
static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
{
uint16_t *r_base, *g_base, *b_base;
@@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
continue;
- drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
funcs->mode_set_base_atomic(mode_set->crtc,
mode_set->fb,
mode_set->x,
@@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
drm_fb_helper *fb_helper,
}
EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
-static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, u16 regno, struct fb_info *info)
-{
- struct drm_fb_helper *fb_helper = info->par;
- struct drm_framebuffer *fb = fb_helper->fb;
-
- if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
- u32 *palette;
- u32 value;
- /* place color in psuedopalette */
- if (regno > 16)
- return -EINVAL;
- palette = (u32 *)info->pseudo_palette;
- red >>= (16 - info->var.red.length);
- green >>= (16 - info->var.green.length);
- blue >>= (16 - info->var.blue.length);
- value = (red << info->var.red.offset) |
- (green << info->var.green.offset) |
- (blue << info->var.blue.offset);
- if (info->var.transp.length > 0) {
- u32 mask = (1 << info->var.transp.length) - 1;
-
- mask <<= info->var.transp.offset;
- value |= mask;
- }
- palette[regno] = value;
- return 0;
- }
-
- /*
-* The driver really shouldn't advertise pseudo/directcolor
-* visuals if it can't deal with the palette.
-*/
- if (WARN_ON(!fb_helper->funcs->gamma_set ||
- !fb_helper->funcs->gamma_get))
- return -EINVAL;
-
- WARN_ON(fb->format->cpp[0] != 1);
-
- fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
-
- return 0;
-}
-
/**
* drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
* @cmap: cmap to set
@@ -1220,51 +1159,61 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
{
struct drm_fb_helper *fb_helper = info->par;
struct drm_device *dev = fb_helper->dev;
- const struct drm_crtc_helper_funcs *crtc_funcs;
- u16 *red, *green, *blue, *transp;
+ struct drm_modeset_acquire_ctx ctx;
struct drm_crtc *crtc;
- int i, j, rc = 0;
- int start;
+ u16 *r, *g, *b;
+ int i, ret;
if (oops_in_progress)
return -EBUSY;
- drm_modeset_lock_all(dev);
+ if (cmap->start + cmap->len < cmap->start)
+ return -EINVAL;
+
+ drm_modeset_acquire_init(&ctx, 0);
+retry:
+ ret = drm_modeset_lock_all_ctx(dev, &ctx);
+ if (ret)
+ goto out;
if (!drm_fb_helper_is_bound(fb_helper)) {
- drm_modeset_unlock_all(dev);
- return -EBUSY;
+ ret = -EBUSY;
+ goto out;
}
for (i = 0; i < fb_helper->crtc_count; i++) {
crtc = fb_helper
Re: [Nouveau] [PATCH 11/11] drm: remove unused and redundant callbacks
On Thu, 22 Jun 2017 08:37:55 +0200 Daniel Vetter wrote: > On Thu, Jun 22, 2017 at 12:34:36AM +0800, kbuild test robot wrote: > > Hi Peter, > > > > [auto build test ERROR on drm/drm-next] > > [also build test ERROR on next-20170621] > > [cannot apply to v4.12-rc6] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > url: > > https://github.com/0day-ci/linux/commits/Peter-Rosin/improve-the-fb_setcmap-helper/20170621-205441 > > base: git://people.freedesktop.org/~airlied/linux.git drm-next > > config: arm-allmodconfig (attached as .config) > > compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 > > reproduce: > > wget > > https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O > > ~/bin/make.cross > > chmod +x ~/bin/make.cross > > # save the attached .config to linux build tree > > make.cross ARCH=arm > > > > All errors (new ones prefixed by >>): > > > > >> drivers/gpu//drm/armada/armada_fbdev.c:121:2: error: unknown field > > >> 'gamma_set' specified in initializer > > .gamma_set = armada_drm_crtc_gamma_set, > > Looks like you've missed at least the armada driver in your conversion? Already fixed in v2 ;-). ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
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 justs saves the gamma lut for later. However,
>> it saves it to the gamma_store which should already be up to date with what
>> .gamma_get would return and is thus a nop. So, zap it.
>
> Removing drm_fb_helper_save_lut_atomic should be a separate patch I
> think.
Then 3 patches would be needed, first some hybrid thing that does it the
old way, but also stores the lut in .gamma_store, then the split-out that
removes drm_fb_helper_save_lut_atomic, then whatever is needed to get
to the desired code. I can certainly do that, but do you want me to?
I.e., the statement that drm_fb_helper_save_lut_atomic is a nop is only
true when (part of) the other patch is also considered.
>> Signed-off-by: Peter Rosin
>
>> ---
>> drivers/gpu/drm/drm_fb_helper.c | 131
>>
>> 1 file changed, 40 insertions(+), 91 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 574af01..cc2d55d 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct
>> drm_fb_helper *fb_helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
>>
>> -static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct
>> drm_fb_helper *helper)
>> -{
>> -uint16_t *r_base, *g_base, *b_base;
>> -int i;
>> -
>> -if (helper->funcs->gamma_get == NULL)
>> -return;
>> -
>> -r_base = crtc->gamma_store;
>> -g_base = r_base + crtc->gamma_size;
>> -b_base = g_base + crtc->gamma_size;
>> -
>> -for (i = 0; i < crtc->gamma_size; i++)
>> -helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i],
>> &b_base[i], i);
>> -}
>> -
>> static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>> {
>> uint16_t *r_base, *g_base, *b_base;
>> @@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
>> if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
>> continue;
>>
>> -drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
>> funcs->mode_set_base_atomic(mode_set->crtc,
>> mode_set->fb,
>> mode_set->x,
>> @@ -1167,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
>> drm_fb_helper *fb_helper,
>> }
>> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>>
>> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>> - u16 blue, u16 regno, struct fb_info *info)
>> -{
>> -struct drm_fb_helper *fb_helper = info->par;
>> -struct drm_framebuffer *fb = fb_helper->fb;
>> -
>> -if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
>
> This case here seems gone, and it was also the pièce de résistance when I
> tried tackling fbdev lut support. As far as I understand this stuff we do
> not support FB_VISUAL_TRUECOLOR palette, and all that bitshifting here is
> pointless. But I'm honestly not really clear.
Oops, sorry, I simply missed that, I'll have a closer look...
> I think removing this case should also be a separate patch, and I'd very
> much prefer that someone with some fbdev-clue would ack it.
>
>> -u32 *palette;
>> -u32 value;
>> -/* place color in psuedopalette */
>> -if (regno > 16)
>> -return -EINVAL;
>> -palette = (u32 *)info->pseudo_palette;
>> -red >>= (16 - info->var.red.length);
>> -green >>= (16 - info->var.green.length);
>> -blue >>= (16 - info->var.blue.length);
>> -value = (red << info->var.red.offset) |
>> -(green << info->var.green.offset) |
>> -(blue << info->var.blue.offset);
>> -if (info->var.transp.length > 0) {
>> -u32 mask = (1 << info->var.transp.length) - 1;
>> -
>> -mask <<= info->var.transp.offset;
>> -value |= mask;
>> -}
>> -palette[regno] = value;
>> -return 0;
>> -}
>> -
>> -/*
>> - * The driver really shouldn't advertise pseudo/directcolor
>> - * visuals if it can't deal with the palette.
>> - */
>> -if (WARN_ON(!fb_helper->funcs->gamma_set ||
>> -!fb_helper->funcs->gamma_get))
>> -return -EINVAL;
>> -
>> -WARN_ON(fb->format->cpp[0] != 1);
>> -
>> -fb_helper-
[Nouveau] [PATCH v2 09/14] drm: i915: remove dead code and pointless local lut storage
The driver stores lut values from the fbdev interface, and is able
to give them back, but does not appear to do anything with these
lut values. The generic fb helpers have replaced this function,
and may even have made the driver work for the C8 mode from the
fbdev interface. But that is untested.
Since the fb helpers .gamma_set and .gamma_get are obsolete,
remove the dead code.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_fbdev.c | 31 ---
2 files changed, 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d93efb4..bc7bfa0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -786,7 +786,6 @@ struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
enum plane plane;
- u8 lut_r[256], lut_g[256], lut_b[256];
/*
* Whether the crtc and the connected output pipeline is active. Implies
* that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6..5bac953 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -281,27 +281,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
return ret;
}
-/** Sets the color ramps on behalf of RandR */
-static void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- intel_crtc->lut_r[regno] = red >> 8;
- intel_crtc->lut_g[regno] = green >> 8;
- intel_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16
*green,
- u16 *blue, int regno)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- *red = intel_crtc->lut_r[regno] << 8;
- *green = intel_crtc->lut_g[regno] << 8;
- *blue = intel_crtc->lut_b[regno] << 8;
-}
-
static struct drm_fb_helper_crtc *
intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
{
@@ -370,7 +349,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_fb_helper_crtc *new_crtc;
- struct intel_crtc *intel_crtc;
fb_conn = fb_helper->connector_info[i];
connector = fb_conn->connector;
@@ -412,13 +390,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
num_connectors_enabled++;
- intel_crtc = to_intel_crtc(connector->state->crtc);
- for (j = 0; j < 256; j++) {
- intel_crtc->lut_r[j] = j;
- intel_crtc->lut_g[j] = j;
- intel_crtc->lut_b[j] = j;
- }
-
new_crtc = intel_fb_helper_crtc(fb_helper,
connector->state->crtc);
@@ -519,8 +490,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
.initial_config = intel_fb_initial_config,
- .gamma_set = intel_crtc_fb_gamma_set,
- .gamma_get = intel_crtc_fb_gamma_get,
.fb_probe = intelfb_create,
};
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
totally obsolete.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/drm_fb_helper.c | 151 +---
1 file changed, 63 insertions(+), 88 deletions(-)
This is an alternative version rebased on top of Daniel's "fbdev helper
locking rework and deferred setup" series.
And as noted by Daniel, .gamma_set does an atomic commit. Thus, the locks
needs to be dropped and reacquired for each crtc. So, that is fixed here
too. Doing it like this with a couple of individual alternative patches
instead of sending a whole new series since the dependency on Daniel's
series makes life somewhat difficult...
Cheers,
peda
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 4aceb59..aa025f1 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1257,50 +1257,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
drm_fb_helper *fb_helper,
}
EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
-static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, u16 regno, struct fb_info *info)
-{
- struct drm_fb_helper *fb_helper = info->par;
- struct drm_framebuffer *fb = fb_helper->fb;
-
- if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
- u32 *palette;
- u32 value;
- /* place color in psuedopalette */
- if (regno > 16)
- return -EINVAL;
- palette = (u32 *)info->pseudo_palette;
- red >>= (16 - info->var.red.length);
- green >>= (16 - info->var.green.length);
- blue >>= (16 - info->var.blue.length);
- value = (red << info->var.red.offset) |
- (green << info->var.green.offset) |
- (blue << info->var.blue.offset);
- if (info->var.transp.length > 0) {
- u32 mask = (1 << info->var.transp.length) - 1;
-
- mask <<= info->var.transp.offset;
- value |= mask;
- }
- palette[regno] = value;
- return 0;
- }
-
- /*
-* The driver really shouldn't advertise pseudo/directcolor
-* visuals if it can't deal with the palette.
-*/
- if (WARN_ON(!fb_helper->funcs->gamma_set ||
- !fb_helper->funcs->gamma_get))
- return -EINVAL;
-
- WARN_ON(fb->format->cpp[0] != 1);
-
- fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
-
- return 0;
-}
-
/**
* drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
* @cmap: cmap to set
@@ -1310,12 +1266,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
{
struct drm_fb_helper *fb_helper = info->par;
struct drm_device *dev = fb_helper->dev;
- const struct drm_crtc_helper_funcs *crtc_funcs;
- u16 *red, *green, *blue, *transp;
+ struct drm_modeset_acquire_ctx ctx;
struct drm_crtc *crtc;
u16 *r, *g, *b;
- int i, j, rc = 0;
- int start;
+ int i, ret = 0;
if (oops_in_progress)
return -EBUSY;
@@ -1329,61 +1283,82 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
return -EBUSY;
}
- drm_modeset_lock_all(dev);
- for (i = 0; i < fb_helper->crtc_count; i++) {
- crtc = fb_helper->crtc_info[i].mode_set.crtc;
- crtc_funcs = crtc->helper_private;
+ drm_modeset_acquire_init(&ctx, 0);
- red = cmap->red;
- green = cmap->green;
- blue = cmap->blue;
- transp = cmap->transp;
- start = cmap->start;
+ for (i = 0; i < fb_helper->crtc_count; i++) {
+ if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+ u32 *palette;
+ int j;
- if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
- if (!crtc->gamma_size) {
- rc = -EINVAL;
- goto out;
+ if (cmap->start + cmap->len > 16) {
+ ret = -EINVAL;
+ break;
}
- if (cmap->start + cmap->len > crtc->gamma_size) {
- rc = -EINVAL;
- goto out;
+ palette = (u32 *)info->pseudo_palette;
+ for (j = 0; j < cmap->len; ++j) {
+ u16 red = cmap->red[j];
+ u16 green = cmap->green[j];
+ u16 blue = cmap->blue[j];
+ u32 value;
+
+ red >>= 16 - info->var.red.length;
+
[Nouveau] [PATCH v2 03/14] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
This makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
totally obsolete.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/drm_fb_helper.c | 154
1 file changed, 63 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7ade384..58eb045 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1150,50 +1150,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
drm_fb_helper *fb_helper,
}
EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
-static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, u16 regno, struct fb_info *info)
-{
- struct drm_fb_helper *fb_helper = info->par;
- struct drm_framebuffer *fb = fb_helper->fb;
-
- if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
- u32 *palette;
- u32 value;
- /* place color in psuedopalette */
- if (regno > 16)
- return -EINVAL;
- palette = (u32 *)info->pseudo_palette;
- red >>= (16 - info->var.red.length);
- green >>= (16 - info->var.green.length);
- blue >>= (16 - info->var.blue.length);
- value = (red << info->var.red.offset) |
- (green << info->var.green.offset) |
- (blue << info->var.blue.offset);
- if (info->var.transp.length > 0) {
- u32 mask = (1 << info->var.transp.length) - 1;
-
- mask <<= info->var.transp.offset;
- value |= mask;
- }
- palette[regno] = value;
- return 0;
- }
-
- /*
-* The driver really shouldn't advertise pseudo/directcolor
-* visuals if it can't deal with the palette.
-*/
- if (WARN_ON(!fb_helper->funcs->gamma_set ||
- !fb_helper->funcs->gamma_get))
- return -EINVAL;
-
- WARN_ON(fb->format->cpp[0] != 1);
-
- fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
-
- return 0;
-}
-
/**
* drm_fb_helper_setcmap - implementation for &fb_ops.fb_setcmap
* @cmap: cmap to set
@@ -1203,12 +1159,10 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
{
struct drm_fb_helper *fb_helper = info->par;
struct drm_device *dev = fb_helper->dev;
- const struct drm_crtc_helper_funcs *crtc_funcs;
- u16 *red, *green, *blue, *transp;
+ struct drm_modeset_acquire_ctx ctx;
struct drm_crtc *crtc;
u16 *r, *g, *b;
- int i, j, rc = 0;
- int start;
+ int i, ret;
if (oops_in_progress)
return -EBUSY;
@@ -1216,65 +1170,83 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
if (cmap->start + cmap->len < cmap->start)
return -EINVAL;
- drm_modeset_lock_all(dev);
+ drm_modeset_acquire_init(&ctx, 0);
+retry:
+ ret = drm_modeset_lock_all_ctx(dev, &ctx);
+ if (ret)
+ goto out;
if (!drm_fb_helper_is_bound(fb_helper)) {
- drm_modeset_unlock_all(dev);
- return -EBUSY;
+ ret = -EBUSY;
+ goto out;
}
for (i = 0; i < fb_helper->crtc_count; i++) {
- crtc = fb_helper->crtc_info[i].mode_set.crtc;
- crtc_funcs = crtc->helper_private;
-
- red = cmap->red;
- green = cmap->green;
- blue = cmap->blue;
- transp = cmap->transp;
- start = cmap->start;
+ if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
+ u32 *palette;
+ int j;
- if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
- if (!crtc->gamma_size) {
- rc = -EINVAL;
+ if (cmap->start + cmap->len > 16) {
+ ret = -EINVAL;
goto out;
}
- if (cmap->start + cmap->len > crtc->gamma_size) {
- rc = -EINVAL;
- goto out;
+ palette = (u32 *)info->pseudo_palette;
+ for (j = 0; j < cmap->len; ++j) {
+ u16 red = cmap->red[j];
+ u16 green = cmap->green[j];
+ u16 blue = cmap->blue[j];
+ u32 value;
+
+ red >>= 16 - info->var.red.length;
+ green >>= 16 - info->var.green.length;
+ blue >>= 16 - info->var.blue.length;
+ value = (red << info->var.red.offset) |
+
[Nouveau] [PATCH v2 08/14] drm: gma500: remove dead code and pointless local lut storage
The redundant fb helpers .gamma_set and .gamma_get are no longer
used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/gma500/framebuffer.c | 22
drivers/gpu/drm/gma500/gma_display.c | 32 ++
drivers/gpu/drm/gma500/psb_intel_display.c | 7 +--
drivers/gpu/drm/gma500/psb_intel_drv.h | 1 -
4 files changed, 12 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/gma500/framebuffer.c
b/drivers/gpu/drm/gma500/framebuffer.c
index 7da70b6..2570c7f 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -479,26 +479,6 @@ static struct drm_framebuffer *psb_user_framebuffer_create
return psb_framebuffer_create(dev, cmd, r);
}
-static void psbfb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
- gma_crtc->lut_r[regno] = red >> 8;
- gma_crtc->lut_g[regno] = green >> 8;
- gma_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void psbfb_gamma_get(struct drm_crtc *crtc, u16 *red,
- u16 *green, u16 *blue, int regno)
-{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
-
- *red = gma_crtc->lut_r[regno] << 8;
- *green = gma_crtc->lut_g[regno] << 8;
- *blue = gma_crtc->lut_b[regno] << 8;
-}
-
static int psbfb_probe(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
@@ -525,8 +505,6 @@ static int psbfb_probe(struct drm_fb_helper *helper,
}
static const struct drm_fb_helper_funcs psb_fb_helper_funcs = {
- .gamma_set = psbfb_gamma_set,
- .gamma_get = psbfb_gamma_get,
.fb_probe = psbfb_probe,
};
diff --git a/drivers/gpu/drm/gma500/gma_display.c
b/drivers/gpu/drm/gma500/gma_display.c
index e7fd356..f3c48a2 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -144,33 +144,32 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
const struct psb_offset *map = &dev_priv->regmap[gma_crtc->pipe];
int palreg = map->palette;
+ u16 *r, *g, *b;
int i;
/* The clocks have to be on to load the palette. */
if (!crtc->enabled)
return;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
if (gma_power_begin(dev, false)) {
for (i = 0; i < 256; i++) {
REG_WRITE(palreg + 4 * i,
- ((gma_crtc->lut_r[i] +
- gma_crtc->lut_adj[i]) << 16) |
- ((gma_crtc->lut_g[i] +
- gma_crtc->lut_adj[i]) << 8) |
- (gma_crtc->lut_b[i] +
- gma_crtc->lut_adj[i]));
+ (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+ (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+ ((*b++ >> 8) + gma_crtc->lut_adj[i]));
}
gma_power_end(dev);
} else {
for (i = 0; i < 256; i++) {
/* FIXME: Why pipe[0] and not pipe[..._crtc->pipe]? */
dev_priv->regs.pipe[0].palette[i] =
- ((gma_crtc->lut_r[i] +
- gma_crtc->lut_adj[i]) << 16) |
- ((gma_crtc->lut_g[i] +
- gma_crtc->lut_adj[i]) << 8) |
- (gma_crtc->lut_b[i] +
- gma_crtc->lut_adj[i]);
+ (((*r++ >> 8) + gma_crtc->lut_adj[i]) << 16) |
+ (((*g++ >> 8) + gma_crtc->lut_adj[i]) << 8) |
+ ((*b++ >> 8) + gma_crtc->lut_adj[i]);
}
}
@@ -180,15 +179,6 @@ int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
u16 *green, u16 *blue,
u32 size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- gma_crtc->lut_r[i] = red[i] >> 8;
- gma_crtc->lut_g[i] = green[i] >> 8;
- gma_crtc->lut_b[i] = blue[i] >> 8;
- }
-
gma_crtc_load_lut(crtc);
return 0;
diff --git a/drivers/gpu/drm/gma500/psb_intel_display.c
b/drivers/gpu/drm/gma500/psb_intel_display.c
index 7b6c849..8762efa 100644
--- a/drivers/gpu/drm/gma500/psb_intel_display.c
+++ b/drivers/gpu/drm/gma500/psb_intel_displ
[Nouveau] [PATCH v2 12/14] drm: radeon: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/radeon/atombios_crtc.c | 1 -
drivers/gpu/drm/radeon/radeon_connectors.c | 7 ++-
drivers/gpu/drm/radeon/radeon_display.c | 71 -
drivers/gpu/drm/radeon/radeon_fb.c | 2 -
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 -
drivers/gpu/drm/radeon/radeon_mode.h| 4 --
6 files changed, 33 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c
b/drivers/gpu/drm/radeon/atombios_crtc.c
index 3c492a0..02baaaf 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -2217,7 +2217,6 @@ static const struct drm_crtc_helper_funcs
atombios_helper_funcs = {
.mode_set_base_atomic = atombios_crtc_set_base_atomic,
.prepare = atombios_crtc_prepare,
.commit = atombios_crtc_commit,
- .load_lut = radeon_crtc_load_lut,
.disable = atombios_crtc_disable,
};
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 27affbd..2f642cb 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -773,12 +773,15 @@ static int radeon_connector_set_property(struct
drm_connector *connector, struct
if (connector->encoder->crtc) {
struct drm_crtc *crtc = connector->encoder->crtc;
- const struct drm_crtc_helper_funcs *crtc_funcs =
crtc->helper_private;
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
radeon_crtc->output_csc = radeon_encoder->output_csc;
- (*crtc_funcs->load_lut)(crtc);
+ /*
+* Our .gamma_set assumes the .gamma_store has been
+* prefilled and don't care about its arguments.
+*/
+ crtc->funcs->gamma_set(crtc, NULL, NULL, NULL, 0, NULL);
}
}
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 17d3daf..8b7d7a0 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x003f);
WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(AVIVO_DC_LUT_30_COLOR,
-(radeon_crtc->lut_r[i] << 20) |
-(radeon_crtc->lut_g[i] << 10) |
-(radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
/* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
@@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset,
0x0007);
WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
- (radeon_crtc->lut_r[i] << 20) |
- (radeon_crtc->lut_g[i] << 10) |
- (radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
}
@@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@
[Nouveau] [PATCH 03/11] drm: ast: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/ast/ast_drv.h | 1 -
drivers/gpu/drm/ast/ast_fb.c | 20
drivers/gpu/drm/ast/ast_mode.c | 26 ++
3 files changed, 6 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 8880f0b..569a148 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -245,7 +245,6 @@ struct ast_connector {
struct ast_crtc {
struct drm_crtc base;
- u8 lut_r[256], lut_g[256], lut_b[256];
struct drm_gem_object *cursor_bo;
uint64_t cursor_addr;
int cursor_width, cursor_height;
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 4ad4acd..dbabcac 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -255,27 +255,7 @@ static int astfb_create(struct drm_fb_helper *helper,
return ret;
}
-static void ast_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- ast_crtc->lut_r[regno] = red >> 8;
- ast_crtc->lut_g[regno] = green >> 8;
- ast_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void ast_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- *red = ast_crtc->lut_r[regno] << 8;
- *green = ast_crtc->lut_g[regno] << 8;
- *blue = ast_crtc->lut_b[regno] << 8;
-}
-
static const struct drm_fb_helper_funcs ast_fb_helper_funcs = {
- .gamma_set = ast_fb_gamma_set,
- .gamma_get = ast_fb_gamma_get,
.fb_probe = astfb_create,
};
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index aaef0a6..724c16b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -63,15 +63,18 @@ static inline void ast_load_palette_index(struct
ast_private *ast,
static void ast_crtc_load_lut(struct drm_crtc *crtc)
{
struct ast_private *ast = crtc->dev->dev_private;
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+ u16 *r, *g, *b;
int i;
if (!crtc->enabled)
return;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++)
- ast_load_palette_index(ast, i, ast_crtc->lut_r[i],
- ast_crtc->lut_g[i], ast_crtc->lut_b[i]);
+ ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
}
static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct
drm_display_mode *mode,
@@ -633,7 +636,6 @@ static const struct drm_crtc_helper_funcs
ast_crtc_helper_funcs = {
.mode_set = ast_crtc_mode_set,
.mode_set_base = ast_crtc_mode_set_base,
.disable = ast_crtc_disable,
- .load_lut = ast_crtc_load_lut,
.prepare = ast_crtc_prepare,
.commit = ast_crtc_commit,
@@ -648,15 +650,6 @@ static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16
*red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- int i;
-
- /* userspace palettes are always correct as is */
- for (i = 0; i < size; i++) {
- ast_crtc->lut_r[i] = red[i] >> 8;
- ast_crtc->lut_g[i] = green[i] >> 8;
- ast_crtc->lut_b[i] = blue[i] >> 8;
- }
ast_crtc_load_lut(crtc);
return 0;
@@ -681,7 +674,6 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
static int ast_crtc_init(struct drm_device *dev)
{
struct ast_crtc *crtc;
- int i;
crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
if (!crtc)
@@ -690,12 +682,6 @@ static int ast_crtc_init(struct drm_device *dev)
drm_crtc_init(dev, &crtc->base, &ast_crtc_funcs);
drm_mode_crtc_set_gamma_size(&crtc->base, 256);
drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
-
- for (i = 0; i < 256; i++) {
- crtc->lut_r[i] = i;
- crtc->lut_g[i] = i;
- crtc->lut_b[i] = i;
- }
return 0;
}
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH 07/11] drm: mgag200: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/mgag200/mgag200_drv.h | 5 ---
drivers/gpu/drm/mgag200/mgag200_fb.c | 2 --
drivers/gpu/drm/mgag200/mgag200_mode.c | 62 --
3 files changed, 15 insertions(+), 54 deletions(-)
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
b/drivers/gpu/drm/mgag200/mgag200_drv.h
index c88b6ec..04f1dfb 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.h
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
@@ -237,11 +237,6 @@ mgag200_bo(struct ttm_buffer_object *bo)
{
return container_of(bo, struct mgag200_bo, bo);
}
- /* mgag200_crtc.c */
-void mga_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, int regno);
-void mga_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-u16 *blue, int regno);
/* mgag200_mode.c */
int mgag200_modeset_init(struct mga_device *mdev);
diff --git a/drivers/gpu/drm/mgag200/mgag200_fb.c
b/drivers/gpu/drm/mgag200/mgag200_fb.c
index 5d3b1fa..5cf980a 100644
--- a/drivers/gpu/drm/mgag200/mgag200_fb.c
+++ b/drivers/gpu/drm/mgag200/mgag200_fb.c
@@ -258,8 +258,6 @@ static int mga_fbdev_destroy(struct drm_device *dev,
}
static const struct drm_fb_helper_funcs mga_fb_helper_funcs = {
- .gamma_set = mga_crtc_fb_gamma_set,
- .gamma_get = mga_crtc_fb_gamma_get,
.fb_probe = mgag200fb_create,
};
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index adb411a..117bec3 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -27,15 +27,19 @@
static void mga_crtc_load_lut(struct drm_crtc *crtc)
{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct mga_device *mdev = dev->dev_private;
struct drm_framebuffer *fb = crtc->primary->fb;
+ u16 *r_ptr, *g_ptr, *b_ptr;
int i;
if (!crtc->enabled)
return;
+ r_ptr = crtc->gamma_store;
+ g_ptr = r_ptr + crtc->gamma_size;
+ b_ptr = g_ptr + crtc->gamma_size;
+
WREG8(DAC_INDEX + MGA1064_INDEX, 0);
if (fb && fb->format->cpp[0] * 8 == 16) {
@@ -46,25 +50,27 @@ static void mga_crtc_load_lut(struct drm_crtc *crtc)
if (i > (MGAG200_LUT_SIZE >> 1)) {
r = b = 0;
} else {
- r = mga_crtc->lut_r[i << 1];
- b = mga_crtc->lut_b[i << 1];
+ r = *r_ptr++ >> 8;
+ b = *b_ptr++ >> 8;
+ r_ptr++;
+ b_ptr++;
}
} else {
- r = mga_crtc->lut_r[i];
- b = mga_crtc->lut_b[i];
+ r = *r_ptr++ >> 8;
+ b = *b_ptr++ >> 8;
}
/* VGA registers */
WREG8(DAC_INDEX + MGA1064_COL_PAL, r);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
WREG8(DAC_INDEX + MGA1064_COL_PAL, b);
}
return;
}
for (i = 0; i < MGAG200_LUT_SIZE; i++) {
/* VGA registers */
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_r[i]);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_g[i]);
- WREG8(DAC_INDEX + MGA1064_COL_PAL, mga_crtc->lut_b[i]);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *r_ptr++ >> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *g_ptr++ >> 8);
+ WREG8(DAC_INDEX + MGA1064_COL_PAL, *b_ptr++ >> 8);
}
}
@@ -1396,14 +1402,6 @@ static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16
*red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- mga_crtc->lut_r[i] = red[i] >> 8;
- mga_crtc->lut_g[i] = green[i] >> 8;
- mga_crtc->lut_b[i] = blue[i] >> 8;
- }
mga_crtc_load_lut(crtc);
return 0;
@@ -1452,14 +1450,12 @@ static const struct drm_crtc_helper_funcs
mga_helper_funcs = {
.mode_set_base = mga_crtc_mode_set_base,
[Nouveau] [PATCH 08/11] drm: nouveau: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/nouveau/dispnv04/crtc.c | 26 --
drivers/gpu/drm/nouveau/nouveau_crtc.h | 3 ---
drivers/gpu/drm/nouveau/nouveau_fbcon.c | 22 ---
drivers/gpu/drm/nouveau/nv50_display.c | 39 ++---
4 files changed, 21 insertions(+), 69 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 4b4b0b4..8f689f1 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -764,13 +764,18 @@ nv_crtc_gamma_load(struct drm_crtc *crtc)
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
struct drm_device *dev = nv_crtc->base.dev;
struct rgb { uint8_t r, g, b; } __attribute__((packed)) *rgbs;
+ u16 *r, *g, *b;
int i;
rgbs = (struct rgb
*)nv04_display(dev)->mode_reg.crtc_reg[nv_crtc->index].DAC;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++) {
- rgbs[i].r = nv_crtc->lut.r[i] >> 8;
- rgbs[i].g = nv_crtc->lut.g[i] >> 8;
- rgbs[i].b = nv_crtc->lut.b[i] >> 8;
+ rgbs[i].r = *r++ >> 8;
+ rgbs[i].g = *g++ >> 8;
+ rgbs[i].b = *b++ >> 8;
}
nouveau_hw_load_state_palette(dev, nv_crtc->index,
&nv04_display(dev)->mode_reg);
@@ -792,13 +797,6 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g,
u16 *b,
struct drm_modeset_acquire_ctx *ctx)
{
struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
- int i;
-
- for (i = 0; i < size; i++) {
- nv_crtc->lut.r[i] = r[i];
- nv_crtc->lut.g[i] = g[i];
- nv_crtc->lut.b[i] = b[i];
- }
/* We need to know the depth before we upload, but it's possible to
* get called before a framebuffer is bound. If this is the case,
@@ -1095,7 +1093,6 @@ static const struct drm_crtc_helper_funcs
nv04_crtc_helper_funcs = {
.mode_set = nv_crtc_mode_set,
.mode_set_base = nv04_crtc_mode_set_base,
.mode_set_base_atomic = nv04_crtc_mode_set_base_atomic,
- .load_lut = nv_crtc_gamma_load,
.disable = nv_crtc_disable,
};
@@ -1103,17 +1100,12 @@ int
nv04_crtc_create(struct drm_device *dev, int crtc_num)
{
struct nouveau_crtc *nv_crtc;
- int ret, i;
+ int ret;
nv_crtc = kzalloc(sizeof(*nv_crtc), GFP_KERNEL);
if (!nv_crtc)
return -ENOMEM;
- for (i = 0; i < 256; i++) {
- nv_crtc->lut.r[i] = i << 8;
- nv_crtc->lut.g[i] = i << 8;
- nv_crtc->lut.b[i] = i << 8;
- }
nv_crtc->lut.depth = 0;
nv_crtc->index = crtc_num;
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h
b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index 050fcf3..b7a18fb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
+++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
@@ -61,9 +61,6 @@ struct nouveau_crtc {
struct {
struct nouveau_bo *nvbo;
- uint16_t r[256];
- uint16_t g[256];
- uint16_t b[256];
int depth;
} lut;
diff --git a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
index 2665a07..f770784 100644
--- a/drivers/gpu/drm/nouveau/nouveau_fbcon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_fbcon.c
@@ -278,26 +278,6 @@ nouveau_fbcon_accel_init(struct drm_device *dev)
info->fbops = &nouveau_fbcon_ops;
}
-static void nouveau_fbcon_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
- nv_crtc->lut.r[regno] = red;
- nv_crtc->lut.g[regno] = green;
- nv_crtc->lut.b[regno] = blue;
-}
-
-static void nouveau_fbcon_gamma_get(struct drm_crtc *crtc, u16 *red, u16
*green,
- u16 *blue, int regno)
-{
- struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-
- *red = nv_crtc->lut.r[regno];
- *green = nv_crtc->lut.g[regno];
- *blue = nv_crtc->lut.b[regno];
-}
-
static void
nouveau_fbcon_zfill(struct drm_device *dev, struct nouveau_fbdev *fbcon)
{
@@ -467,8 +447,6 @@ void nouveau_fbcon_gpu_lockup(struct fb_info *info)
}
static const struct drm_fb_helper_funcs nouveau_fbcon_helper_funcs = {
- .gamma_set = nouveau_fbcon_gamma_set,
- .gamma_get = nouveau_fbcon_gamma_get,
.fb_probe = nouveau_fbcon_create,
};
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
b/drivers/gpu/drm/nouveau/nv50_display.c
index 775c1
[Nouveau] [PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
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?
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/drm_fb_helper.c | 27 +++
1 file changed, 27 insertions(+)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 574af01..25315fb 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1223,12 +1223,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
const struct drm_crtc_helper_funcs *crtc_funcs;
u16 *red, *green, *blue, *transp;
struct drm_crtc *crtc;
+ u16 *r, *g, *b;
int i, j, rc = 0;
int start;
if (oops_in_progress)
return -EBUSY;
+ if (cmap->start + cmap->len < cmap->start)
+ return -EINVAL;
+
drm_modeset_lock_all(dev);
if (!drm_fb_helper_is_bound(fb_helper)) {
drm_modeset_unlock_all(dev);
@@ -1245,6 +1249,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
transp = cmap->transp;
start = cmap->start;
+ if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+ if (!crtc->gamma_size) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (cmap->start + cmap->len > crtc->gamma_size) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
+ memcpy(r + cmap->start, cmap->red,
+ cmap->len * sizeof(u16));
+ memcpy(g + cmap->start, cmap->green,
+ cmap->len * sizeof(u16));
+ memcpy(b + cmap->start, cmap->blue,
+ cmap->len * sizeof(u16));
+ }
+
for (j = 0; j < cmap->len; j++) {
u16 hred, hgreen, hblue, htransp = 0x;
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 06/14] drm: ast: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/ast/ast_drv.h | 1 -
drivers/gpu/drm/ast/ast_fb.c | 20
drivers/gpu/drm/ast/ast_mode.c | 26 ++
3 files changed, 6 insertions(+), 41 deletions(-)
diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h
index 8880f0b..569a148 100644
--- a/drivers/gpu/drm/ast/ast_drv.h
+++ b/drivers/gpu/drm/ast/ast_drv.h
@@ -245,7 +245,6 @@ struct ast_connector {
struct ast_crtc {
struct drm_crtc base;
- u8 lut_r[256], lut_g[256], lut_b[256];
struct drm_gem_object *cursor_bo;
uint64_t cursor_addr;
int cursor_width, cursor_height;
diff --git a/drivers/gpu/drm/ast/ast_fb.c b/drivers/gpu/drm/ast/ast_fb.c
index 4ad4acd..dbabcac 100644
--- a/drivers/gpu/drm/ast/ast_fb.c
+++ b/drivers/gpu/drm/ast/ast_fb.c
@@ -255,27 +255,7 @@ static int astfb_create(struct drm_fb_helper *helper,
return ret;
}
-static void ast_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- ast_crtc->lut_r[regno] = red >> 8;
- ast_crtc->lut_g[regno] = green >> 8;
- ast_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void ast_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno)
-{
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- *red = ast_crtc->lut_r[regno] << 8;
- *green = ast_crtc->lut_g[regno] << 8;
- *blue = ast_crtc->lut_b[regno] << 8;
-}
-
static const struct drm_fb_helper_funcs ast_fb_helper_funcs = {
- .gamma_set = ast_fb_gamma_set,
- .gamma_get = ast_fb_gamma_get,
.fb_probe = astfb_create,
};
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index aaef0a6..724c16b 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -63,15 +63,18 @@ static inline void ast_load_palette_index(struct
ast_private *ast,
static void ast_crtc_load_lut(struct drm_crtc *crtc)
{
struct ast_private *ast = crtc->dev->dev_private;
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
+ u16 *r, *g, *b;
int i;
if (!crtc->enabled)
return;
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
for (i = 0; i < 256; i++)
- ast_load_palette_index(ast, i, ast_crtc->lut_r[i],
- ast_crtc->lut_g[i], ast_crtc->lut_b[i]);
+ ast_load_palette_index(ast, i, *r++ >> 8, *g++ >> 8, *b++ >> 8);
}
static bool ast_get_vbios_mode_info(struct drm_crtc *crtc, struct
drm_display_mode *mode,
@@ -633,7 +636,6 @@ static const struct drm_crtc_helper_funcs
ast_crtc_helper_funcs = {
.mode_set = ast_crtc_mode_set,
.mode_set_base = ast_crtc_mode_set_base,
.disable = ast_crtc_disable,
- .load_lut = ast_crtc_load_lut,
.prepare = ast_crtc_prepare,
.commit = ast_crtc_commit,
@@ -648,15 +650,6 @@ static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16
*red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
- int i;
-
- /* userspace palettes are always correct as is */
- for (i = 0; i < size; i++) {
- ast_crtc->lut_r[i] = red[i] >> 8;
- ast_crtc->lut_g[i] = green[i] >> 8;
- ast_crtc->lut_b[i] = blue[i] >> 8;
- }
ast_crtc_load_lut(crtc);
return 0;
@@ -681,7 +674,6 @@ static const struct drm_crtc_funcs ast_crtc_funcs = {
static int ast_crtc_init(struct drm_device *dev)
{
struct ast_crtc *crtc;
- int i;
crtc = kzalloc(sizeof(struct ast_crtc), GFP_KERNEL);
if (!crtc)
@@ -690,12 +682,6 @@ static int ast_crtc_init(struct drm_device *dev)
drm_crtc_init(dev, &crtc->base, &ast_crtc_funcs);
drm_mode_crtc_set_gamma_size(&crtc->base, 256);
drm_crtc_helper_add(&crtc->base, &ast_crtc_helper_funcs);
-
- for (i = 0; i < 256; i++) {
- crtc->lut_r[i] = i;
- crtc->lut_g[i] = i;
- crtc->lut_b[i] = i;
- }
return 0;
}
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 05/14] drm: armada: remove dead empty functions
The redundant fb helpers .gamma_set and .gamma_get are no longer used.
Remove the dead code.
Signed-off-by: Peter Rosin
---
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 4fe19fd..96bccf8 100644
--- a/drivers/gpu/drm/armada/armada_crtc.c
+++ b/drivers/gpu/drm/armada/armada_crtc.c
@@ -334,16 +334,6 @@ static void armada_drm_vblank_off(struct armada_crtc
*dcrtc)
armada_drm_plane_work_run(dcrtc, dcrtc->crtc.primary);
}
-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 7e8906d..bab11f4 100644
--- a/drivers/gpu/drm/armada/armada_crtc.h
+++ b/drivers/gpu/drm/armada/armada_crtc.h
@@ -102,8 +102,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_update_regs(struct armada_crtc *, struct armada_regs *);
void armada_drm_crtc_plane_disable(struct armada_crtc *dcrtc,
diff --git a/drivers/gpu/drm/armada/armada_fbdev.c
b/drivers/gpu/drm/armada/armada_fbdev.c
index 602dfea..5fa076d 100644
--- a/drivers/gpu/drm/armada/armada_fbdev.c
+++ b/drivers/gpu/drm/armada/armada_fbdev.c
@@ -118,8 +118,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,
};
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 04/14] drm: amd: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c | 24
drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 -
drivers/gpu/drm/amd/amdgpu/dce_v10_0.c | 27 +++
drivers/gpu/drm/amd/amdgpu/dce_v11_0.c | 27 +++
drivers/gpu/drm/amd/amdgpu/dce_v6_0.c| 27 +++
drivers/gpu/drm/amd/amdgpu/dce_v8_0.c| 27 +++
drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 23 ---
7 files changed, 28 insertions(+), 128 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index c0d8c6f..7dc3780 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -312,31 +312,7 @@ static int amdgpu_fbdev_destroy(struct drm_device *dev,
struct amdgpu_fbdev *rfb
return 0;
}
-/** Sets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
- amdgpu_crtc->lut_r[regno] = red >> 6;
- amdgpu_crtc->lut_g[regno] = green >> 6;
- amdgpu_crtc->lut_b[regno] = blue >> 6;
-}
-
-/** Gets the color ramps on behalf of fbcon */
-static void amdgpu_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16
*green,
- u16 *blue, int regno)
-{
- struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-
- *red = amdgpu_crtc->lut_r[regno] << 6;
- *green = amdgpu_crtc->lut_g[regno] << 6;
- *blue = amdgpu_crtc->lut_b[regno] << 6;
-}
-
static const struct drm_fb_helper_funcs amdgpu_fb_helper_funcs = {
- .gamma_set = amdgpu_crtc_fb_gamma_set,
- .gamma_get = amdgpu_crtc_fb_gamma_get,
.fb_probe = amdgpufb_create,
};
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 43a9d3a..39f7eda 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -369,7 +369,6 @@ struct amdgpu_atom_ss {
struct amdgpu_crtc {
struct drm_crtc base;
int crtc_id;
- u16 lut_r[256], lut_g[256], lut_b[256];
bool enabled;
bool can_tile;
uint32_t crtc_offset;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 9f78c03..c958023 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2267,6 +2267,7 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc *crtc)
struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct amdgpu_device *adev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
u32 tmp;
@@ -2304,11 +2305,14 @@ static void dce_v10_0_crtc_load_lut(struct drm_crtc
*crtc)
WREG32(mmDC_LUT_WRITE_EN_MASK + amdgpu_crtc->crtc_offset, 0x0007);
WREG32(mmDC_LUT_RW_INDEX + amdgpu_crtc->crtc_offset, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(mmDC_LUT_30_COLOR + amdgpu_crtc->crtc_offset,
- (amdgpu_crtc->lut_r[i] << 20) |
- (amdgpu_crtc->lut_g[i] << 10) |
- (amdgpu_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
tmp = RREG32(mmDEGAMMA_CONTROL + amdgpu_crtc->crtc_offset);
@@ -2624,15 +2628,6 @@ static int dce_v10_0_crtc_gamma_set(struct drm_crtc
*crtc, u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
- int i;
-
- /* userspace palettes are always correct as is */
- for (i = 0; i < size; i++) {
- amdgpu_crtc->lut_r[i] = red[i] >> 6;
- amdgpu_crtc->lut_g[i] = green[i] >> 6;
- amdgpu_crtc->lut_b[i] = blue[i] >> 6;
- }
dce_v10_0_crtc_load_lut(crtc);
return 0;
@@ -2844,14 +2839,12 @@ static const struct drm_crtc_helper_funcs
dce_v10_0_crtc_helper_funcs = {
.mode_set_base_atomic = dce_v10_0_crtc_set_base_atomic,
.prepare = dce_v10_0_crtc_prepare,
.commit = dce_v10_0_crtc_commit,
- .load_lut = dce_v10_0_crtc_load_lut,
.disable = dce_v10_0_crtc_disable,
};
static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int ind
[Nouveau] [PATCH 09/11] drm: radeon: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/radeon/atombios_crtc.c | 1 -
drivers/gpu/drm/radeon/radeon_connectors.c | 7 ++-
drivers/gpu/drm/radeon/radeon_display.c | 71 -
drivers/gpu/drm/radeon/radeon_fb.c | 2 -
drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 1 -
5 files changed, 33 insertions(+), 49 deletions(-)
diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c
b/drivers/gpu/drm/radeon/atombios_crtc.c
index 3c492a0..02baaaf 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -2217,7 +2217,6 @@ static const struct drm_crtc_helper_funcs
atombios_helper_funcs = {
.mode_set_base_atomic = atombios_crtc_set_base_atomic,
.prepare = atombios_crtc_prepare,
.commit = atombios_crtc_commit,
- .load_lut = radeon_crtc_load_lut,
.disable = atombios_crtc_disable,
};
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c
b/drivers/gpu/drm/radeon/radeon_connectors.c
index 27affbd..2f642cb 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -773,12 +773,15 @@ static int radeon_connector_set_property(struct
drm_connector *connector, struct
if (connector->encoder->crtc) {
struct drm_crtc *crtc = connector->encoder->crtc;
- const struct drm_crtc_helper_funcs *crtc_funcs =
crtc->helper_private;
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
radeon_crtc->output_csc = radeon_encoder->output_csc;
- (*crtc_funcs->load_lut)(crtc);
+ /*
+* Our .gamma_set assumes the .gamma_store has been
+* prefilled and don't care about its arguments.
+*/
+ crtc->funcs->gamma_set(crtc, NULL, NULL, NULL, 0, NULL);
}
}
diff --git a/drivers/gpu/drm/radeon/radeon_display.c
b/drivers/gpu/drm/radeon/radeon_display.c
index 17d3daf..8b7d7a0 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -42,6 +42,7 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -60,11 +61,14 @@ static void avivo_crtc_load_lut(struct drm_crtc *crtc)
WREG32(AVIVO_DC_LUT_WRITE_EN_MASK, 0x003f);
WREG8(AVIVO_DC_LUT_RW_INDEX, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(AVIVO_DC_LUT_30_COLOR,
-(radeon_crtc->lut_r[i] << 20) |
-(radeon_crtc->lut_g[i] << 10) |
-(radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
/* Only change bit 0 of LUT_SEL, other bits are set elsewhere */
@@ -76,6 +80,7 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -93,11 +98,14 @@ static void dce4_crtc_load_lut(struct drm_crtc *crtc)
WREG32(EVERGREEN_DC_LUT_WRITE_EN_MASK + radeon_crtc->crtc_offset,
0x0007);
WREG32(EVERGREEN_DC_LUT_RW_INDEX + radeon_crtc->crtc_offset, 0);
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
for (i = 0; i < 256; i++) {
WREG32(EVERGREEN_DC_LUT_30_COLOR + radeon_crtc->crtc_offset,
- (radeon_crtc->lut_r[i] << 20) |
- (radeon_crtc->lut_g[i] << 10) |
- (radeon_crtc->lut_b[i] << 0));
+ ((*r++ & 0xffc0) << 14) |
+ ((*g++ & 0xffc0) << 4) |
+ (*b++ >> 6));
}
}
@@ -106,6 +114,7 @@ static void dce5_crtc_load_lut(struct drm_crtc *crtc)
struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct radeon_device *rdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
DRM_DEBUG_KMS("%d\n", radeon_crtc->crtc_id);
@@ -135,11 +144,14 @@ static void dce5_crtc_load_lut(str
Re: [Nouveau] [PATCH 11/11] drm: remove unused and redundant callbacks
Hi Peter,
[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20170621]
[cannot apply to v4.12-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Peter-Rosin/improve-the-fb_setcmap-helper/20170621-205441
base: git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget
https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
>> drivers/gpu//drm/armada/armada_fbdev.c:121:2: error: unknown field
>> 'gamma_set' specified in initializer
.gamma_set = armada_drm_crtc_gamma_set,
^
>> drivers/gpu//drm/armada/armada_fbdev.c:121:15: error: initialization from
>> incompatible pointer type [-Werror=incompatible-pointer-types]
.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,
^
drivers/gpu//drm/armada/armada_fbdev.c:122:15: note: (near initialization
for 'armada_fb_helper_funcs.initial_config')
cc1: some warnings being treated as errors
vim +/gamma_set +121 drivers/gpu//drm/armada/armada_fbdev.c
96f60e37 Russell King 2012-08-15 115 ret = 1;
96f60e37 Russell King 2012-08-15 116 }
96f60e37 Russell King 2012-08-15 117 return ret;
96f60e37 Russell King 2012-08-15 118 }
96f60e37 Russell King 2012-08-15 119
3a493879 Thierry Reding 2014-06-27 120 static const struct
drm_fb_helper_funcs armada_fb_helper_funcs = {
96f60e37 Russell King 2012-08-15 @121 .gamma_set =
armada_drm_crtc_gamma_set,
96f60e37 Russell King 2012-08-15 @122 .gamma_get =
armada_drm_crtc_gamma_get,
96f60e37 Russell King 2012-08-15 123 .fb_probe =
armada_fb_probe,
96f60e37 Russell King 2012-08-15 124 };
96f60e37 Russell King 2012-08-15 125
:: The code at line 121 was first introduced by commit
:: 96f60e37dc66091bde8d5de136ff6fda09f2d799 DRM: Armada: Add Armada DRM
driver
:: TO: Russell King
:: CC: Russell King
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 02/14] drm/fb-helper: remove drm_fb_helper_save_lut_atomic
drm_fb_helper_save_lut_atomic is redundant since the .gamma_store is
now always kept up to date by drm_fb_helper_setcmap.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/drm_fb_helper.c | 17 -
1 file changed, 17 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 25315fb..7ade384 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -229,22 +229,6 @@ int drm_fb_helper_remove_one_connector(struct
drm_fb_helper *fb_helper,
}
EXPORT_SYMBOL(drm_fb_helper_remove_one_connector);
-static void drm_fb_helper_save_lut_atomic(struct drm_crtc *crtc, struct
drm_fb_helper *helper)
-{
- uint16_t *r_base, *g_base, *b_base;
- int i;
-
- if (helper->funcs->gamma_get == NULL)
- return;
-
- r_base = crtc->gamma_store;
- g_base = r_base + crtc->gamma_size;
- b_base = g_base + crtc->gamma_size;
-
- for (i = 0; i < crtc->gamma_size; i++)
- helper->funcs->gamma_get(crtc, &r_base[i], &g_base[i],
&b_base[i], i);
-}
-
static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
{
uint16_t *r_base, *g_base, *b_base;
@@ -285,7 +269,6 @@ int drm_fb_helper_debug_enter(struct fb_info *info)
if (drm_drv_uses_atomic_modeset(mode_set->crtc->dev))
continue;
- drm_fb_helper_save_lut_atomic(mode_set->crtc, helper);
funcs->mode_set_base_atomic(mode_set->crtc,
mode_set->fb,
mode_set->x,
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On 2017-06-22 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? drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. However, it saves it to the gamma_store which should already be up to date with what .gamma_get would return and is thus a nop. So, zap it. >>> >>> Removing drm_fb_helper_save_lut_atomic should be a separate patch I >>> think. >> >> Then 3 patches would be needed, first some hybrid thing that does it the >> old way, but also stores the lut in .gamma_store, then the split-out that >> removes drm_fb_helper_save_lut_atomic, then whatever is needed to get >> to the desired code. I can certainly do that, but do you want me to? > > Explain that in the commit message and it's fine. I did the split in v2, I assume that's ok too. Better in case anyone ever needs to run a bisect on this... >>> It's a pre-existing bug, but should we also try to restore the fbdev lut >>> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug, >>> but might be relevant for your use-case. Just try to run both an fbdev >>> application and some kms-native thing, and then SIGKILL the native kms >>> app. >>> >>> But since pre-existing not really required, and probably too much effort. >> >> Good thing too, because I don't really know my way around this code... > > Btw I cc'ed you on one of my patches in the fbdev locking series, we might > need to do the same legacy vs. atomic split for the new lut code as I did > for dpms. The rule with atomic is that you can't do multiple commits under > drm_modeset_lock_all, you either have to do one overall atomic commit > (preferred) or drop&reacquire locks again. This matters for LUT since > you're updating the LUT on all CRTCs, which when using the gamma_set > atomic helper would be multiple commits :-/ Ahh, ok, I see the problem. > Using the dpms patch as template it shouldn't be too hard to address that > for your patch here too. Hmm, in that patch you handle the legacy case in a separate function, and doing that for the lut case looks difficult when the atomic commit happens inside the helper (typically drm_atomic_helper_legacy_gamma_set which could perhaps be handled, but a real drag to handle for drivers that have a custom crtc .gamma_set). So, I'm aiming for the drop&reacquire approach... However, I don't have all of that series, and I suspect that is why I do not have any fb_helper->lock. I'll send my best guess as a follow-up to patch 3/14 in v2. Cheers, peda ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH 04/11] drm: cirrus: remove dead code and pointless local lut storage
The redundant fb helpers .load_lut, .gamma_set and .gamma_get are
no longer used. Remove the dead code and hook up the crtc .gamma_set
to use the crtc gamma_store directly instead of duplicating that
info locally.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/cirrus/cirrus_drv.h | 8
drivers/gpu/drm/cirrus/cirrus_fbdev.c | 2 -
drivers/gpu/drm/cirrus/cirrus_mode.c | 71 ---
3 files changed, 16 insertions(+), 65 deletions(-)
diff --git a/drivers/gpu/drm/cirrus/cirrus_drv.h
b/drivers/gpu/drm/cirrus/cirrus_drv.h
index 8690352..be2d7e48 100644
--- a/drivers/gpu/drm/cirrus/cirrus_drv.h
+++ b/drivers/gpu/drm/cirrus/cirrus_drv.h
@@ -96,7 +96,6 @@
struct cirrus_crtc {
struct drm_crtc base;
- u8 lut_r[256], lut_g[256], lut_b[256];
int last_dpms;
boolenabled;
};
@@ -180,13 +179,6 @@ cirrus_bo(struct ttm_buffer_object *bo)
#define to_cirrus_obj(x) container_of(x, struct cirrus_gem_object, base)
#define DRM_FILE_PAGE_OFFSET (0x1ULL >> PAGE_SHIFT)
- /* cirrus_mode.c */
-void cirrus_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
-u16 blue, int regno);
-void cirrus_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
-u16 *blue, int regno);
-
-
/* cirrus_main.c */
int cirrus_device_init(struct cirrus_device *cdev,
struct drm_device *ddev,
diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index 7fa58ee..1fedab0 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -265,8 +265,6 @@ static int cirrus_fbdev_destroy(struct drm_device *dev,
}
static const struct drm_fb_helper_funcs cirrus_fb_helper_funcs = {
- .gamma_set = cirrus_crtc_fb_gamma_set,
- .gamma_get = cirrus_crtc_fb_gamma_get,
.fb_probe = cirrusfb_create,
};
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c
b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 53f6f0f..a4c4a46 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -31,25 +31,6 @@
* This file contains setup code for the CRTC.
*/
-static void cirrus_crtc_load_lut(struct drm_crtc *crtc)
-{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
- struct drm_device *dev = crtc->dev;
- struct cirrus_device *cdev = dev->dev_private;
- int i;
-
- if (!crtc->enabled)
- return;
-
- for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
- /* VGA registers */
- WREG8(PALETTE_INDEX, i);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_r[i]);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_g[i]);
- WREG8(PALETTE_DATA, cirrus_crtc->lut_b[i]);
- }
-}
-
/*
* The DRM core requires DPMS functions, but they make little sense in our
* case and so are just stubs
@@ -330,15 +311,25 @@ static int cirrus_crtc_gamma_set(struct drm_crtc *crtc,
u16 *red, u16 *green,
u16 *blue, uint32_t size,
struct drm_modeset_acquire_ctx *ctx)
{
- struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
+ struct drm_device *dev = crtc->dev;
+ struct cirrus_device *cdev = dev->dev_private;
+ u16 *r, *g, *b;
int i;
- for (i = 0; i < size; i++) {
- cirrus_crtc->lut_r[i] = red[i];
- cirrus_crtc->lut_g[i] = green[i];
- cirrus_crtc->lut_b[i] = blue[i];
+ if (!crtc->enabled)
+ return 0;
+
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
+ for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+ /* VGA registers */
+ WREG8(PALETTE_INDEX, i);
+ WREG8(PALETTE_DATA, *r++ >> 8);
+ WREG8(PALETTE_DATA, *g++ >> 8);
+ WREG8(PALETTE_DATA, *b++ >> 8);
}
- cirrus_crtc_load_lut(crtc);
return 0;
}
@@ -365,7 +356,6 @@ static const struct drm_crtc_helper_funcs
cirrus_helper_funcs = {
.mode_set_base = cirrus_crtc_mode_set_base,
.prepare = cirrus_crtc_prepare,
.commit = cirrus_crtc_commit,
- .load_lut = cirrus_crtc_load_lut,
};
/* CRTC setup */
@@ -373,7 +363,6 @@ static void cirrus_crtc_init(struct drm_device *dev)
{
struct cirrus_device *cdev = dev->dev_private;
struct cirrus_crtc *cirrus_crtc;
- int i;
cirrus_crtc = kzalloc(sizeof(struct cirrus_crtc) +
(CIRRUSFB_CONN_LIMIT * sizeof(struct
drm_connector *)),
@@ -387,37 +376,9 @@ static void cirrus_crtc_init(struct drm_device *dev)
drm_mode_crtc_set_gamma_size(&cirrus_crtc->base, CIRRUS_LUT_SIZE);
cde
[Nouveau] [PATCH 06/11] drm: i915: remove dead code and pointless local lut storage
The driver stores lut values from the fbdev interface, and is able
to give them back, but does not appear to do anything with these
lut values. The generic fb helpers have replaced this function,
and may even have made the driver work for the C8 mode from the
fbdev interface. But that is untested.
Since the fb helper .gamma_set and .gamma_get are no longer used,
just kill the mysterious dead code.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/i915/intel_drv.h | 1 -
drivers/gpu/drm/i915/intel_fbdev.c | 31 ---
2 files changed, 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 83dd409..503edf3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -785,7 +785,6 @@ struct intel_crtc {
struct drm_crtc base;
enum pipe pipe;
enum plane plane;
- u8 lut_r[256], lut_g[256], lut_b[256];
/*
* Whether the crtc and the connected output pipeline is active. Implies
* that crtc->enabled is set, i.e. the current mode configuration has
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c
b/drivers/gpu/drm/i915/intel_fbdev.c
index 03347c6..5bac953 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -281,27 +281,6 @@ static int intelfb_create(struct drm_fb_helper *helper,
return ret;
}
-/** Sets the color ramps on behalf of RandR */
-static void intel_crtc_fb_gamma_set(struct drm_crtc *crtc, u16 red, u16 green,
- u16 blue, int regno)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- intel_crtc->lut_r[regno] = red >> 8;
- intel_crtc->lut_g[regno] = green >> 8;
- intel_crtc->lut_b[regno] = blue >> 8;
-}
-
-static void intel_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16
*green,
- u16 *blue, int regno)
-{
- struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
- *red = intel_crtc->lut_r[regno] << 8;
- *green = intel_crtc->lut_g[regno] << 8;
- *blue = intel_crtc->lut_b[regno] << 8;
-}
-
static struct drm_fb_helper_crtc *
intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc)
{
@@ -370,7 +349,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
struct drm_connector *connector;
struct drm_encoder *encoder;
struct drm_fb_helper_crtc *new_crtc;
- struct intel_crtc *intel_crtc;
fb_conn = fb_helper->connector_info[i];
connector = fb_conn->connector;
@@ -412,13 +390,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
num_connectors_enabled++;
- intel_crtc = to_intel_crtc(connector->state->crtc);
- for (j = 0; j < 256; j++) {
- intel_crtc->lut_r[j] = j;
- intel_crtc->lut_g[j] = j;
- intel_crtc->lut_b[j] = j;
- }
-
new_crtc = intel_fb_helper_crtc(fb_helper,
connector->state->crtc);
@@ -519,8 +490,6 @@ static bool intel_fb_initial_config(struct drm_fb_helper
*fb_helper,
static const struct drm_fb_helper_funcs intel_fb_helper_funcs = {
.initial_config = intel_fb_initial_config,
- .gamma_set = intel_crtc_fb_gamma_set,
- .gamma_get = intel_crtc_fb_gamma_get,
.fb_probe = intelfb_create,
};
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 13/14] drm: stm: remove dead code and pointless local lut storage
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.
Just remove the dead code.
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/stm/ltdc.c | 12
drivers/gpu/drm/stm/ltdc.h | 1 -
2 files changed, 13 deletions(-)
diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index 1b9483d..87829b9 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -375,17 +375,6 @@ static irqreturn_t ltdc_irq(int irq, void *arg)
* DRM_CRTC
*/
-static void ltdc_crtc_load_lut(struct drm_crtc *crtc)
-{
- struct ltdc_device *ldev = crtc_to_ltdc(crtc);
- unsigned int i, lay;
-
- for (lay = 0; lay < ldev->caps.nb_layers; lay++)
- for (i = 0; i < 256; i++)
- reg_write(ldev->regs, LTDC_L1CLUTWR + lay * LAY_OFS,
- ldev->clut[i]);
-}
-
static void ltdc_crtc_enable(struct drm_crtc *crtc)
{
struct ltdc_device *ldev = crtc_to_ltdc(crtc);
@@ -523,7 +512,6 @@ static void ltdc_crtc_atomic_flush(struct drm_crtc *crtc,
}
static struct drm_crtc_helper_funcs ltdc_crtc_helper_funcs = {
- .load_lut = ltdc_crtc_load_lut,
.enable = ltdc_crtc_enable,
.disable = ltdc_crtc_disable,
.mode_set_nofb = ltdc_crtc_mode_set_nofb,
diff --git a/drivers/gpu/drm/stm/ltdc.h b/drivers/gpu/drm/stm/ltdc.h
index d7a9c73..620ca55 100644
--- a/drivers/gpu/drm/stm/ltdc.h
+++ b/drivers/gpu/drm/stm/ltdc.h
@@ -27,7 +27,6 @@ struct ltdc_device {
struct drm_panel *panel;
struct mutex err_lock; /* protecting error_status */
struct ltdc_caps caps;
- u32 clut[256]; /* color look up table */
u32 error_status;
u32 irq_status;
};
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 01/14] drm/fb-helper: keep the .gamma_store updated in drm_fb_helper_setcmap
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?
Signed-off-by: Peter Rosin
---
drivers/gpu/drm/drm_fb_helper.c | 27 +++
1 file changed, 27 insertions(+)
This is an alternative version rebased on top of Daniels "fbdev helper
locking rework and deferred setup" series.
Cheers,
peda
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a4cfef9..c7122c9 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1330,12 +1330,16 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
const struct drm_crtc_helper_funcs *crtc_funcs;
u16 *red, *green, *blue, *transp;
struct drm_crtc *crtc;
+ u16 *r, *g, *b;
int i, j, rc = 0;
int start;
if (oops_in_progress)
return -EBUSY;
+ if (cmap->start + cmap->len < cmap->start)
+ return -EINVAL;
+
mutex_lock(&fb_helper->lock);
if (!drm_fb_helper_is_bound(fb_helper)) {
mutex_unlock(&fb_helper->lock);
@@ -1353,6 +1357,29 @@ int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct
fb_info *info)
transp = cmap->transp;
start = cmap->start;
+ if (info->fix.visual != FB_VISUAL_TRUECOLOR) {
+ if (!crtc->gamma_size) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ if (cmap->start + cmap->len > crtc->gamma_size) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ r = crtc->gamma_store;
+ g = r + crtc->gamma_size;
+ b = g + crtc->gamma_size;
+
+ memcpy(r + cmap->start, cmap->red,
+ cmap->len * sizeof(u16));
+ memcpy(g + cmap->start, cmap->green,
+ cmap->len * sizeof(u16));
+ memcpy(b + cmap->start, cmap->blue,
+ cmap->len * sizeof(u16));
+ }
+
for (j = 0; j < cmap->len; j++) {
u16 hred, hgreen, hblue, htransp = 0x;
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [PATCH v2 14/14] drm: remove unused and redundant callbacks
Drivers no longer have any need for these callbacks, and there are no
users. Zap. Zap-zap-zzzap-p-pp-p.
Signed-off-by: Peter Rosin
---
include/drm/drm_fb_helper.h | 32
include/drm/drm_modeset_helper_vtables.h | 16
2 files changed, 48 deletions(-)
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 119e5e4..80d9853 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -85,38 +85,6 @@ struct drm_fb_helper_surface_size {
*/
struct drm_fb_helper_funcs {
/**
-* @gamma_set:
-*
-* Set the given gamma LUT register on the given CRTC.
-*
-* This callback is optional.
-*
-* FIXME:
-*
-* 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 exists because the fbdev hasn't yet been
-* refactored to use the core gamma table interfaces.
-*/
- void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
- u16 *blue, int regno);
-
- /**
* @fb_probe:
*
* Driver callback to allocate and initialize the fbdev info structure.
diff --git a/include/drm/drm_modeset_helper_vtables.h
b/include/drm/drm_modeset_helper_vtables.h
index 85984b2..0773db9 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -267,22 +267,6 @@ struct drm_crtc_helper_funcs {
enum mode_set_atomic);
/**
-* @load_lut:
-*
-* Load a LUT prepared with the &drm_fb_helper_funcs.gamma_set vfunc.
-*
-* This callback is optional and is only used by the fbdev emulation
-* helpers.
-*
-* FIXME:
-*
-* 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 (*load_lut)(struct drm_crtc *crtc);
-
- /**
* @disable:
*
* This callback should be used to disable the CRTC. With the atomic
--
2.1.4
___
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau
[Nouveau] [Bug 101368] Nouveau regression GT218M in Kernel 4.11 Won't Boot
https://bugs.freedesktop.org/show_bug.cgi?id=101368 --- Comment #12 from Ben Steel --- Yikes. I only waited about 28 hours before completely restoring that machine from backup and putting it back in service. Food for thought: my greatest fear is that the problem may have been illuminated by the change to GCC 7, which doesn't like compiling 4.10 kernels. -- You are receiving this mail because: You are the assignee for the bug.___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] [PATCH 01/11] drm/fb-helper: do a generic fb_setcmap helper in terms of crtc .gamma_set
On Thu, Jun 22, 2017 at 10:48:10AM +0200, Peter Rosin wrote: > On 2017-06-22 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? > > drm_fb_helper_save_lut_atomic justs saves the gamma lut for later. > However, > it saves it to the gamma_store which should already be up to date with > what > .gamma_get would return and is thus a nop. So, zap it. > >>> > >>> Removing drm_fb_helper_save_lut_atomic should be a separate patch I > >>> think. > >> > >> Then 3 patches would be needed, first some hybrid thing that does it the > >> old way, but also stores the lut in .gamma_store, then the split-out that > >> removes drm_fb_helper_save_lut_atomic, then whatever is needed to get > >> to the desired code. I can certainly do that, but do you want me to? > > > > Explain that in the commit message and it's fine. > > I did the split in v2, I assume that's ok too. Better in case anyone ever > needs to run a bisect on this... > > >>> It's a pre-existing bug, but should we also try to restore the fbdev lut > >>> in drm_fb_helper_restore_fbdev_mode_unlocked()? Would be yet another bug, > >>> but might be relevant for your use-case. Just try to run both an fbdev > >>> application and some kms-native thing, and then SIGKILL the native kms > >>> app. > >>> > >>> But since pre-existing not really required, and probably too much effort. > >> > >> Good thing too, because I don't really know my way around this code... > > > > Btw I cc'ed you on one of my patches in the fbdev locking series, we might > > need to do the same legacy vs. atomic split for the new lut code as I did > > for dpms. The rule with atomic is that you can't do multiple commits under > > drm_modeset_lock_all, you either have to do one overall atomic commit > > (preferred) or drop&reacquire locks again. This matters for LUT since > > you're updating the LUT on all CRTCs, which when using the gamma_set > > atomic helper would be multiple commits :-/ > > Ahh, ok, I see the problem. > > > Using the dpms patch as template it shouldn't be too hard to address that > > for your patch here too. > > Hmm, in that patch you handle the legacy case in a separate function, and > doing that for the lut case looks difficult when the atomic commit happens > inside the helper (typically drm_atomic_helper_legacy_gamma_set which > could perhaps be handled, but a real drag to handle for drivers that have > a custom crtc .gamma_set). Yeah, that's essentially why you need 2 functions. Legacy one calls the legacy interfaces, the atomic one calls the new fancy atomic property stuff directly (i.e. it open-codes the property setting dance from the helper, but with one atomic commit across all crtc). The reason that the legacy callback functions are helpers and not just default behavior is that I expected some drivers to need special hacks to keep their existing legacy kms userspace working. Atomic helpers unified behaviour a lot that beforehand was slightly inconsistent. I somewhat expect that long-term we'll unexport all the compat helpers and just make them the default for all atomic drivers. > So, I'm aiming for the drop&reacquire approach... Hm, I prefer the open-coding, that's at least what we do everywhere else. Has the benefit that it's more atomic (one commit for everything). > However, I don't have all of that series, and I suspect that is why I do > not have any fb_helper->lock. Oh sorry, entire series is on dri-devel. I can do a git branch too if you need one, atm it's in my general pile: https://cgit.freedesktop.org/~danvet/drm/log/ Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau
