Re: [Nouveau] [PATCH v4 02/17] drm/nouveau/kms/nv50-: Move AUX adapter reg to connector late register/early unregister
Some trivia, no comment on the real logic of the changes:
On Fri, Apr 23, 2021 at 2:43 PM Lyude Paul wrote:
>
> Since AUX adapters on nouveau have their respective DRM connectors as
> parents, we need to make sure that we register then after their connectors.
then -> them
>
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 25 -
> 1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 61e6d7412505..c04044be3d32 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -401,7 +401,6 @@ nouveau_connector_destroy(struct drm_connector *connector)
> drm_connector_cleanup(connector);
> if (nv_connector->aux.transfer) {
> drm_dp_cec_unregister_connector(&nv_connector->aux);
> - drm_dp_aux_unregister(&nv_connector->aux);
> kfree(nv_connector->aux.name);
> }
> kfree(connector);
> @@ -905,13 +904,29 @@ nouveau_connector_late_register(struct drm_connector
> *connector)
> int ret;
>
> ret = nouveau_backlight_init(connector);
> + if (ret)
> + return ret;
>
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> + connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> + ret = drm_dp_aux_register(&nouveau_connector(connector)->aux);
> + if (ret)
> + goto backlight_fini;
> + }
> +
> + return 0;
> +backlight_fini:
> + nouveau_backlight_fini(connector);
> return ret;
> }
>
> static void
> nouveau_connector_early_unregister(struct drm_connector *connector)
> {
> + if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> + connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
> + drm_dp_aux_unregister(&nouveau_connector(connector)->aux);
> +
> nouveau_backlight_fini(connector);
> }
>
> @@ -1343,14 +1358,14 @@ nouveau_connector_create(struct drm_device *dev,
> snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
> dcbe->hasht, dcbe->hashm);
> nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
> - ret = drm_dp_aux_register(&nv_connector->aux);
> + drm_dp_aux_init(&nv_connector->aux);
> if (ret) {
> - NV_ERROR(drm, "failed to register aux channel\n");
> + NV_ERROR(drm, "Failed to init AUX adapter for
> sor-%04x-%04x: %d\n",
Maybe just use aux_name instead of rebuilding the string again?
> +dcbe->hasht, dcbe->hashm, ret);
> kfree(nv_connector);
> return ERR_PTR(ret);
> }
> - funcs = &nouveau_connector_funcs;
> - break;
> + fallthrough;
> default:
> funcs = &nouveau_connector_funcs;
> break;
> --
> 2.30.2
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau/kms/nv50-: Check plane size for cursors, not fb size
On Thu, Mar 18, 2021 at 5:56 PM Lyude Paul wrote:
>
> Found this while trying to make some changes to the kms_cursor_crc test.
> curs507a_acquire checks that the width and height of the cursor framebuffer
> are equal (asyw->image.{w,h}). This is actually wrong though, as we only
> want to be concerned that the actual width/height of the plane are the
> same. It's fine if we scan out from an fb that's slightly larger than the
> cursor plane (in fact, some igt tests actually do this).
How so? The scanout engine expects the data to be packed. Height can
be larger, but width has to match.
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [RFC v3 05/10] drm/i915/dpcd_bl: Cleanup intel_dp_aux_vesa_enable_backlight() a bit
On Fri, Feb 5, 2021 at 6:45 PM Lyude Paul wrote:
>
> Get rid of the extraneous switch case in here, and just open code
> edp_backlight_mode as we only ever use it once.
>
> Signed-off-by: Lyude Paul
> ---
> .../gpu/drm/i915/display/intel_dp_aux_backlight.c | 15 ++-
> 1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index c37ccc8538cb..95e3e344cf40 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -382,7 +382,7 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> struct intel_panel *panel = &connector->panel;
> - u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode;
> + u8 dpcd_buf, new_dpcd_buf;
> u8 pwmgen_bit_count = panel->backlight.edp.vesa.pwmgen_bit_count;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> @@ -393,12 +393,8 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
> }
>
> new_dpcd_buf = dpcd_buf;
> - edp_backlight_mode = dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
>
> - switch (edp_backlight_mode) {
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_PWM:
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET:
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT:
> + if ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) !=
> DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) {
You probably meant != MODE_DPCD?
> new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
> new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
>
> @@ -406,13 +402,6 @@ intel_dp_aux_vesa_enable_backlight(const struct
> intel_crtc_state *crtc_state,
>pwmgen_bit_count) != 1)
> drm_dbg_kms(&i915->drm,
> "Failed to write aux pwmgen bit count\n");
> -
> - break;
> -
> - /* Do nothing when it is already DPCD mode */
> - case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD:
> - default:
> - break;
> }
>
> if (panel->backlight.edp.vesa.pwm_freq_pre_divider) {
> --
> 2.29.2
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
On Tue, Feb 23, 2021 at 9:26 AM Alex Riesen
wrote:
>
> Lyude Paul, Tue, Jan 19, 2021 02:54:13 +0100:
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > index c6367035970e..5f4f09a601d4 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > @@ -2663,6 +2663,14 @@ nv50_display_create(struct drm_device *dev)
> > else
> > nouveau_display(dev)->format_modifiers = disp50xx_modifiers;
> >
> > + if (disp->disp->object.oclass >= GK104_DISP) {
> > + dev->mode_config.cursor_width = 256;
> > + dev->mode_config.cursor_height = 256;
> > + } else {
> > + dev->mode_config.cursor_width = 64;
> > + dev->mode_config.cursor_height = 64;
> > + }
> > +
> > /* create crtc objects to represent the hw heads */
> > if (disp->disp->object.oclass >= GV100_DISP)
> > crtcs = nvif_rd32(&device->object, 0x610060) & 0xff;
>
> This change broke X cursor in my setup, and reverting the commit restores it.
>
> Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1).
> libdrm 2.4.91-1 (Debian 10.8 stable).
> There are no errors or warnings in Xorg logs nor in the kernel log.
Hi Alex,
Could you confirm which ddx is driving the nvidia hw? You can find
this out by running "xrandr --listproviders", or also in the xorg log.
Thanks,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
On Tue, Feb 23, 2021 at 10:36 AM Alex Riesen
wrote:
>
> Ilia Mirkin, Tue, Feb 23, 2021 15:56:21 +0100:
> > On Tue, Feb 23, 2021 at 9:26 AM Alex Riesen
> > wrote:
> > > Lyude Paul, Tue, Jan 19, 2021 02:54:13 +0100:
> > > > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > > > b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > > > index c6367035970e..5f4f09a601d4 100644
> > > > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > > > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > > > @@ -2663,6 +2663,14 @@ nv50_display_create(struct drm_device *dev)
> > > > else
> > > > nouveau_display(dev)->format_modifiers =
> > > > disp50xx_modifiers;
> > > >
> > > > + if (disp->disp->object.oclass >= GK104_DISP) {
> > > > + dev->mode_config.cursor_width = 256;
> > > > + dev->mode_config.cursor_height = 256;
> > > > + } else {
> > > > + dev->mode_config.cursor_width = 64;
> > > > + dev->mode_config.cursor_height = 64;
> > > > + }
> > > > +
> > > > /* create crtc objects to represent the hw heads */
> > > > if (disp->disp->object.oclass >= GV100_DISP)
> > > > crtcs = nvif_rd32(&device->object, 0x610060) & 0xff;
> > >
> > > This change broke X cursor in my setup, and reverting the commit restores
> > > it.
> > >
> > > Dell Precision M4800, issue ~2014 with GK106GLM [Quadro K2100M] (rev a1).
> > > libdrm 2.4.91-1 (Debian 10.8 stable).
> > > There are no errors or warnings in Xorg logs nor in the kernel log.
> >
> > Could you confirm which ddx is driving the nvidia hw? You can find
> > this out by running "xrandr --listproviders", or also in the xorg log.
>
> xrandr(1) does not seem to list much:
>
> $ xrandr --listproviders
> Providers: number : 1
> Provider 0: id: 0x48 cap: 0xf, Source Output, Sink Output, Source Offload,
> Sink Offload crtcs: 4 outputs: 5 associated providers: 0 name:modesetting
Thanks - this is what I was looking for. name:modesetting, i.e. the
modesetting ddx driver.
I checked nouveau source, and it seems like it uses a 64x64 cursor no
matter what. Not sure what the modesetting ddx does.
I'd recommend using xf86-video-nouveau in any case, but some distros
have decided to explicitly force modesetting in preference of nouveau.
Oh well. (And regardless, the regression should be addressed somehow,
but it's also good to understand what the problem is.)
Can you confirm what the problem with the cursor is?
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
On Tue, Feb 23, 2021 at 11:23 AM Alex Riesen wrote: > > Alex Riesen, Tue, Feb 23, 2021 16:51:26 +0100: > > Ilia Mirkin, Tue, Feb 23, 2021 16:46:52 +0100: > > > I'd recommend using xf86-video-nouveau in any case, but some distros > > > > I would like try this out. Do you know how to force the xorg server to > > choose this driver instead of modesetting? > > Found that myself (a Device section with Driver set to "nouveau"): > > $ xrandr --listproviders > Providers: number : 1 > Provider 0: id: 0x68 cap: 0x7, Source Output, Sink Output, Source Offload > crtcs: 4 outputs: 5 associated providers: 0 name:nouveau > > And yes, the cursor looks good in v5.11 even without reverting the commit. FWIW it's not immediately apparent to me what grave error modesetting is committing in setting the cursor. The logic looks perfectly reasonable. It's not trying to be fancy with rendering the cursor/etc. The one thing is that it's using drmModeSetCursor2 which sets the hotspot at the same time. But internally inside nouveau I think it should work out to the same thing. Perhaps setting the hotspot, or something in that path, doesn't quite work for 256x256? [Again, no clue what that might be.] It might also be worthwhile just testing if the 256x256 cursor works quite the way one would want. If you're interested, grab libdrm, there's a test called 'modetest', which has an option to enable a moving cursor (-c iirc). It's hard-coded to 64x64, so you'll have to modify it there too (and probably change the pattern from plain gray to any one of the other ones). Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
[+emersion, -various people and lists who definitely don't care] On Wed, Feb 24, 2021 at 4:09 AM Alex Riesen wrote: > > Ilia Mirkin, Tue, Feb 23, 2021 19:13:59 +0100: > > On Tue, Feb 23, 2021 at 11:23 AM Alex Riesen > > wrote: > > > > > > $ xrandr --listproviders > > > Providers: number : 1 > > > Provider 0: id: 0x68 cap: 0x7, Source Output, Sink Output, Source > > > Offload crtcs: 4 outputs: 5 associated providers: 0 name:nouveau > > > > > > And yes, the cursor looks good in v5.11 even without reverting the commit. > > > > FWIW it's not immediately apparent to me what grave error modesetting > > is committing in setting the cursor. The logic looks perfectly > > reasonable. It's not trying to be fancy with rendering the cursor/etc. > > > > The one thing is that it's using drmModeSetCursor2 which sets the > > hotspot at the same time. But internally inside nouveau I think it > > should work out to the same thing. Perhaps setting the hotspot, or > > something in that path, doesn't quite work for 256x256? [Again, no > > clue what that might be.] > > > > It might also be worthwhile just testing if the 256x256 cursor works > > quite the way one would want. If you're interested, grab libdrm, > > there's a test called 'modetest', which has an option to enable a > > moving cursor (-c iirc). It's hard-coded to 64x64, so you'll have to > > modify it there too (and probably change the pattern from plain gray > > to any one of the other ones). > > I am interested, so I did. > > If I start the test without X running, the sprite of 256x256 cursor always > contained horizontal lines across it, both with commit reverted and vanilla > v5.11. Similarly, the 64x64 cursor has no lines across it in both kernels. > > The test does not seem to work at all if there is an X server running (using > modesetting driver): modetest complained about permission denied to set the > mode, and just sits there, drawing nothing on the displays. > So I could not run the test in the environment of original problem. > Am I starting it correctly? Is the change in modetest.c correct? Looks right. Although TBH I'd just start it on a single display (I forgot you could even do multiple displays). You should be able to start it with the X server running if you switch to a vt (and run it as root). If you can't, that means the modesetting driver is forgetting to do something in the LeaveVT function. The fact that you're getting lines with modetest means there's something wrong with 256x256. What if you do 128x128 -- does that work OK? Simon, you tested on a GK208, that has a slightly later display controller than the GK104 -- can you try to reproduce Alex's results? Perhaps there was a problem with the GK10x's and it starts working OK with the GK110 disp. I don't have any GK10x's in my posession (I have nearly every other iteration of the display controller), but hopefully someone on the nouveau team will be able to dig one up and reproduce. Thanks for testing, Alex! > > $ ./modetest -c |grep '^[0-9]\|preferred' > 85 86 connected LVDS-1 340x190 13 86 > #0 1920x1080 60.01 1920 2010 2070 2226 1080 1086 1095 1142 152540 > flags: phsync, nvsync; type: preferred, driver > 87 89 connected DP-1470x300 18 88, 89 > #0 1680x1050 59.88 1680 1728 1760 1840 1050 1053 1059 1080 119000 > flags: phsync, nvsync; type: preferred, driver > 90 0 disconnectedDP-20x0 0 91, 92 > 93 95 connected DP-3520x320 10 94, 95 > #0 1920x1200 59.95 1920 1968 2000 2080 1200 1203 1209 1235 154000 > flags: phsync, nvsync; type: preferred, driver > 96 0 disconnectedVGA-1 0x0 0 97 > > $ ./modetest -s 85:1920x1080 -s 93:1920x1200 -s 87:1680x1050 -C > trying to open device 'i915'...failed > trying to open device 'amdgpu'...failed > trying to open device 'radeon'...failed > trying to open device 'nouveau'...done > setting mode 1920x1080-60.01Hz on connectors 85, crtc 50 > starting cursor > > cursor stopped > > This is the change on top of 1225171b (master): > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c > index fc75383a..cdba7b4e 100644 > --- a/tests/modetest/modetest.c > +++ b/tests/modetest/modetest.c > @@ -1730,14 +1730,14 @@ static void set_cursors(struct device *dev, struct > pipe_arg *pipes, unsi
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
On Wed, Feb 24, 2021 at 11:35 AM Alex Riesen wrote: > Ilia Mirkin, Wed, Feb 24, 2021 16:10:57 +0100: > > The fact that you're getting lines with modetest means there's > > something wrong with 256x256. What if you do 128x128 -- does that work > > OK? > > Yes. Unfortunately in both cases. Just to be crystal clear -- are you saying that 128x128 works or does not work? (You said "yes", which would imply it works OK, but then you said both cases, which would imply doesn't work since 256x256 doesn't work?) Thanks, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
On Wed, Feb 24, 2021 at 11:53 AM Alex Riesen wrote: > > Ilia Mirkin, Wed, Feb 24, 2021 17:48:39 +0100: > > On Wed, Feb 24, 2021 at 11:35 AM Alex Riesen > > wrote: > > > Ilia Mirkin, Wed, Feb 24, 2021 16:10:57 +0100: > > > > The fact that you're getting lines with modetest means there's > > > > something wrong with 256x256. What if you do 128x128 -- does that work > > > > OK? > > > > > > Yes. Unfortunately in both cases. > > > > Just to be crystal clear -- are you saying that 128x128 works or does > > not work? (You said "yes", which would imply it works OK, but then you > > said both cases, which would imply doesn't work since 256x256 doesn't > > work?) > > Modetest with 128x128 cursor works. Without damage to the cursor: modetest > shows normal cursor in vanilla v5.11. Modetest also shows normal cursor in > vanilla v5.11 with the commit reverted. But modetest with 256x256 doesn't work (correctly) right? Or did I misunderstand? All the patch does is allow those large cursors to be used, which gets reported via drm APIs and modesetting picks the largest cursor available. (And actually I think it's even not required to use the large cursors, it just controls what's reported in the defaults to userspace.) Thanks, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/nouveau/kms/nv50-: Report max cursor size to userspace
On Wed, Feb 24, 2021 at 12:03 PM Alex Riesen wrote: > > Ilia Mirkin, Wed, Feb 24, 2021 17:57:41 +0100: > > On Wed, Feb 24, 2021 at 11:53 AM Alex Riesen > > wrote: > > > Ilia Mirkin, Wed, Feb 24, 2021 17:48:39 +0100: > > > > Just to be crystal clear -- are you saying that 128x128 works or does > > > > not work? (You said "yes", which would imply it works OK, but then you > > > > said both cases, which would imply doesn't work since 256x256 doesn't > > > > work?) > > > > > > Modetest with 128x128 cursor works. Without damage to the cursor: modetest > > > shows normal cursor in vanilla v5.11. Modetest also shows normal cursor in > > > vanilla v5.11 with the commit reverted. > > > > But modetest with 256x256 doesn't work (correctly) right? Or did I > > misunderstand? > > Right. That's why I was asking if I did everything right: it was just > corrupted > in both kernels. OK. So 128x128 works, 256x256 does not. Interesting. > > > All the patch does is allow those large cursors to be used, which gets > > reported via drm APIs and modesetting picks the largest cursor > > available. (And actually I think it's even not required to use the > > large cursors, it just controls what's reported in the defaults to > > userspace.) > > Maybe something in X code is not prepared to handle the kernel reporting > large cursor support? Even though 128x128 is pretty large, and I don't think > I even use that large cursors in X configuration. How can I check? Yes, 64x64 is enough for anyone (or was it 640kb?) But it's unlikely to be an issue. I believe that AMD also exposes 256x256 cursors depending on the gen: display/dc/dce100/dce100_resource.c:dc->caps.max_cursor_size = 128; display/dc/dce110/dce110_resource.c:dc->caps.max_cursor_size = 128; display/dc/dce112/dce112_resource.c:dc->caps.max_cursor_size = 128; display/dc/dce120/dce120_resource.c:dc->caps.max_cursor_size = 128; display/dc/dce60/dce60_resource.c: dc->caps.max_cursor_size = 64; display/dc/dce60/dce60_resource.c: dc->caps.max_cursor_size = 64; display/dc/dce60/dce60_resource.c: dc->caps.max_cursor_size = 64; display/dc/dce80/dce80_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce80/dce80_resource.c: dc->caps.max_cursor_size = 128; display/dc/dce80/dce80_resource.c: dc->caps.max_cursor_size = 128; display/dc/dcn10/dcn10_resource.c: dc->caps.max_cursor_size = 256; display/dc/dcn20/dcn20_resource.c: dc->caps.max_cursor_size = 256; display/dc/dcn21/dcn21_resource.c: dc->caps.max_cursor_size = 256; display/dc/dcn30/dcn30_resource.c: dc->caps.max_cursor_size = 256; which should have the equivalent effect. But since you're seeing issues with modetest as well (which uses the ioctl's pretty directly), presumably Xorg is not to blame. It's easy enough to adjust the kernel to report a lower size (and reject the larger cursors), I just want to understand which gens are affected by this. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 18/18] drm/i915/display13: Enabling dithering after the CC1 pipe
Just wanted to mention ... nouveau supports two separate properties, one controlling the type of dithering, and the other the dithering depth: dithering depth: auto supported: auto, 6 bpc, 8 bpc dithering mode: auto supported: auto, off, static 2x2, dynamic 2x2, temporal I think these are the properties Mario was alluding to. If I have an 8bpc buffer and set the dithering depth to 6bpc, it will dither down to this. This is useful for LVDS panels primarily. Sometimes we know it's 6bpc (and "auto" works), sometimes we don't. These properties are largely a reflection of how the hardware works. For example, https://nvidia.github.io/open-gpu-doc/classes/display/cl917d.h search for SET_DITHER_CONTROL. Perhaps this "API" would not be appropriate for Intel HW, not sure. But there's definitely precedent. Cheers, -ilia On Sun, Feb 28, 2021 at 11:57 PM Varide, Nischal wrote: > > Looks like there are two options. > > Enable or Disable Dithering via kernel command line or sysfs. > To implement new Uapi. > > May be the first one is more feasible and faster > > > > Regards > > Nischal > > > > From: Mario Kleiner > Sent: Friday, February 19, 2021 11:15 AM > To: Ville Syrjälä > Cc: Roper, Matthew D ; intel-gfx > ; Varide, Nischal > ; dri-devel > Subject: Re: [Intel-gfx] [PATCH 18/18] drm/i915/display13: Enabling dithering > after the CC1 pipe > > > > > > > > On Fri, Feb 19, 2021 at 4:22 AM Mario Kleiner > wrote: > > > > > > On Thu, Feb 11, 2021 at 1:29 PM Ville Syrjälä > wrote: > > On Thu, Jan 28, 2021 at 11:24:13AM -0800, Matt Roper wrote: > > From: Nischal Varide > > > > If the panel is 12bpc then Dithering is not enabled in the Legacy > > dithering block , instead its Enabled after the C1 CC1 pipe post > > color space conversion.For a 6bpc pannel Dithering is enabled in > > Legacy block. > > Dithering is probably going to require a whole uapi bikeshed. > Not sure we can just enable it unilaterally. > > Ccing dri-devel, and Mario who had issues with dithering in the > past... > > Thanks for the cc Ville! > > > > The problem with dithering on Intel is that various tested Intel gpu's > (Ironlake, IvyBridge, Haswell, Skylake iirc.) are dithering when they > shouldn't. If one has a standard 8 bpc framebuffer feeding into a standard > (legacy) 256 slots, 8 bit wide lut which was loaded with an identity mapping, > feeding into a standard 8 bpc video output (DVI/HDMI/DP), the expected result > is that pixels rendered into the framebuffer show up unmodified at the video > output. What happens instead is that some dithering is needlessly applied. > This is bad for various neuroscience/medical research equipment that requires > pixels to pass unmodified in a pure 8 bpc configuration, e.g., because some > digital info is color-encoded in-band in the rendered image to control > research hardware, a la "if rgb pixel (123, 12, 23) is detected in the > digital video stream, emit some trigger signal, or timestamp that moment with > a hw clock, or start or stop some scientific recording equipment". Also there > exist specialized visual stimulators to drive special displays with more than > 12 bpc, e.g., 16 bpc, and so they encode the 8MSB of 16 bpc color values in > pixels in even columns, and the 8LSB in the odd columns of the framebuffer. > Unexpected dithering makes such equipment completely unusable. By now I must > have spent months of my life, just trying to deal with dithering induced > problems on different gpu's due to hw quirks or bugs somewhere in the > graphics stack. > > > > Atm. the intel kms driver disables dithering for anything with >= 8 bpc as a > fix for this harmful hardware quirk. > > > > Ideally we'd have uapi that makes dithering controllable per connector > (on/off/auto, selectable depth), also in a way that those controls are > exposed as RandR output properties, easily controllable by X clients. And > some safe default in case the client can't access the properties (like I'd > expect to happen with the dozens of Wayland compositors under the sun). > Various drivers had this over time, e.g., AMD classic kms path (if i don't > misremember) and nouveau, but some of it also got lost in the new atomic kms > variants, and Intel never exposed this. > > > > Or maybe some method that checks the values actually stored in the hw lut's, > CTM etc. and if the values suggest no dithering should be needed, disable the > dithering. E.g., if output depth is 8 bpc, one only needs dithering if the > slots in the final active hw lut do have any meaningful values in the lower > bits below the top 8 MSB, ie. if the content is actually > 8 bpc net bit > depth. > > > > -mario > > > > > > One cup of coffee later... I think this specific patch should be ok wrt. my > use cases. The majority of the above mentioned research devices are > single/dual-link DVI receivers, ie. 8 bpc video sinks. I'm only aware of one > recent device that has
[PATCH] drm/nouveau/kms/nv04: use vzalloc for nv04_display
The struct is giant, and triggers an order-7 allocation (512K). There is no reason for this to be kmalloc-type memory, so switch to vmalloc. This should help loading nouveau on low-memory and/or long-running systems. Reported-by: Nathan E. Egge Signed-off-by: Ilia Mirkin Cc: sta...@vger.kernel.org --- drivers/gpu/drm/nouveau/dispnv04/disp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c b/drivers/gpu/drm/nouveau/dispnv04/disp.c index 7739f46470d3..99fee4d8cd31 100644 --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c @@ -205,7 +205,7 @@ nv04_display_destroy(struct drm_device *dev) nvif_notify_dtor(&disp->flip); nouveau_display(dev)->priv = NULL; - kfree(disp); + vfree(disp); nvif_object_unmap(&drm->client.device.object); } @@ -223,7 +223,7 @@ nv04_display_create(struct drm_device *dev) struct nv04_display *disp; int i, ret; - disp = kzalloc(sizeof(*disp), GFP_KERNEL); + disp = vzalloc(sizeof(*disp)); if (!disp) return -ENOMEM; -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 3/3] drm: Add GUD USB Display driver
On Thu, Mar 11, 2021 at 3:02 PM Peter Stuge wrote:
> > > Hence the question: What does DRM promise about the XRGB mode?
> >
> > That it's a 32-bit value. From include/uapi/drm/drm_fourcc.h:
> >
> > /* 32 bpp RGB */
> > #define DRM_FORMAT_XRGB fourcc_code('X', 'R', '2', '4') /* [31:0]
> > x:R:G:B 8:8:8:8 little endian */
>
> Okay, "[31:0] x:R:G:B 8:8:8:8" can certainly mean
> [31:24]=x [23:16]=R [15:8]=G [7:0]=B, which when stored "little endian"
> becomes B G R X in memory, for which your pix32 code is correct.
>
> That's the reverse *memory* layout of what the name says :) but yes,
> the name then matches the representation seen by software. That's the
> "abstracted" case that I didn't expect, because I thought the name was
> refering to memory layout and because I was thinking about how traditional
> graphics adapter video memory has the R component at the lower
> address, at least in early linear modes.
The definition of the formats is memory layout in little endian. The
definition you see is of a 32-bit packed little-endian integer, which
is a fixed memory layout.
Now, if you're on an actual big-endian platform, and you want to
accept big-endian-packed formats, there's a bit of unpleasantness that
goes on. Basically there are two options:
1. Ignore the above definition and interpret the formats as
*big-endian* layouts. This is what nouveau and radeon do. They also
don't support AddFB2 (which is what allows supplying a format) -- only
AddFB which just has depth (and bpp). That's fine for nouveau and
radeon because the relevant userspace just uses AddFB, and knows what
the drivers want, so it all works out.
2. Comply with the above definition and set
dev->mode_config.quirk_addfb_prefer_host_byte_order to false. This
loses you native host packing of RGB565/etc, since they're just not
defined as formats. There's a DRM_FORMAT_BIG_ENDIAN bit but it's not
properly supported for anything but the formats.
I'm not sure why you guys were talking about BE in the first place,
but since this is a topic I've looked into (in the context of moving
nouveau from 1 to 2 - but that can't happen due to the reduced format
availability), figured I'd share some of the current sad state.
Cheers,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 3/3] drm: Add GUD USB Display driver
On Thu, Mar 11, 2021 at 5:58 PM Peter Stuge wrote:
>
> Ilia Mirkin wrote:
> > > > #define DRM_FORMAT_XRGB fourcc_code('X', 'R', '2', '4') /* [31:0]
> > > > x:R:G:B 8:8:8:8 little endian */
> > >
> > > Okay, "[31:0] x:R:G:B 8:8:8:8" can certainly mean
> > > [31:24]=x [23:16]=R [15:8]=G [7:0]=B, which when stored "little endian"
> > > becomes B G R X in memory, for which your pix32 code is correct.
> > >
> > > That's the reverse *memory* layout of what the name says :)
> >
> > The definition of the formats is memory layout in little endian.
>
> To clarify, my new (hopefully correct?) understanding is this:
>
> XRGB does *not* mean that address 0=X, 1=R, 2=G, 3=B, but that
> the most significant byte in a packed XRGB 32-bit integer is X
> and the least significant byte is B, and that this is the case both
> on LE and BE machines.
Not quite.
XRGB means that the memory layout should match a 32-bit integer,
stored as LE, with the low bits being B, next bits being G, etc. This
translates to byte 0 = B, byte 1 = G, etc. If you're on a BE system,
and you're handed a XRGB buffer, it still expects that byte 0 = B,
etc (except as I outlined, some drivers which are from before these
formats were a thing, sort of do their own thing). Thankfully this is
equivalent to BGRX (big-endian packing), so you can just munge the
format. Not so with e.g. RGB565 though (since the components don't
fall on byte boundaries).
> I previously thought that XRGB indicated the memory byte order of
> components being X R G B regardless of machine endianess, but now
> understand XRGB to mean the MSB..LSB order of component bytes within
> the 32-bit integer, as seen by software, not the order of bytes in memory.
There are about 100 conventions, and they all manage to be different
from each other. Packed vs array. BE vs LE. If you're *not* confused,
that should be a red flag.
[...]
> > I'm not sure why you guys were talking about BE in the first place,
>
> I was worried that the translation didn't consider endianess.
The translation in gud_xrgb_to_color definitely seems suspect.
There's also a gud_is_big_endian, but I'm guessing this applies to the
downstream device rather than the host system. I didn't check if
dev->mode_config.quirk_addfb_prefer_host_byte_order is set -- that
setting dictates whether these formats are in host-byte-order (and
AddFB2 is disabled, so buffers can only be specified with depth/bpp
and ambiguous component orders) or in LE byte order (and userspace can
use AddFB2 which gives allows precise formats for these buffers). Not
100% sure what something like Xorg's modesetting driver does, TBH.
This is a very poorly-tested scenario.
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] Nouveau video --- [ cut here ] ----- crash dump 5.10.0-rc6
Unfortunately this isn't a crash, but rather a warning that things are timing out. By the time you get this, the display is most likely hung. Was there anything before this, e.g. an error state dump perhaps? What GPU are you using, what displays, and how are they connected? What kind of userspace is running here? X or a Wayland compositor (or something else entirely)? On Thu, Dec 3, 2020 at 12:13 AM Dave Airlie wrote: > > cc'ing Ben + nouveau > > On Thu, 3 Dec 2020 at 14:59, bob wrote: > > > > Hello. I have a crash dump for: > > > > $ uname -a > > Linux freedom 5.10.0-rc6 #1 SMP Sun Nov 29 17:26:13 MST 2020 x86_64 > > x86_64 x86_64 GNU/Linux > > > > Occasionally when this dumps it likes to lock up the computer, but I > > caught it this time. > > > > Also video likes to flicker a lot. Nouveau has been iffy since kernel > > 5.8.0. > > > > This isn't the only dump, it dumped probably 50 times. If you are > > really desperate for all of it, > > > > reply to me directly as I'm not on the mailing list. Here is one of them. > > > > [39019.426580] [ cut here ] > > [39019.426589] WARNING: CPU: 6 PID: 14136 at > > drivers/gpu/drm/nouveau/dispnv50/disp.c:211 nv50_dmac_wait+0x1e1/0x230 > > [39019.426590] Modules linked in: mt2131 s5h1409 fuse tda8290 tuner > > cx25840 rt2800usb rt2x00usb rt2800lib snd_hda_codec_analog > > snd_hda_codec_generic ledtrig_audio rt2x00lib binfmt_misc > > intel_powerclamp coretemp cx23885 mac80211 tda18271 altera_stapl > > videobuf2_dvb m88ds3103 tveeprom cx2341x dvb_core rc_core i2c_mux > > snd_hda_codec_hdmi videobuf2_dma_sg videobuf2_memops videobuf2_v4l2 > > snd_hda_intel videobuf2_common snd_intel_dspcfg kvm_intel snd_hda_codec > > videodev snd_hda_core kvm mc snd_hwdep snd_pcm_oss snd_mixer_oss > > irqbypass snd_pcm cfg80211 snd_seq_dummy snd_seq_midi snd_seq_oss > > snd_seq_midi_event snd_rawmidi snd_seq intel_cstate snd_seq_device > > serio_raw snd_timer input_leds nfsd libarc4 snd asus_atk0110 i7core_edac > > soundcore i5500_temp auth_rpcgss nfs_acl lockd grace sch_fq_codel sunrpc > > parport_pc ppdev lp parport ip_tables x_tables btrfs blake2b_generic > > libcrc32c xor zstd_compress raid6_pq dm_mirror dm_region_hash dm_log > > pata_acpi pata_marvell hid_generic usbhid hid psmouse firewire_ohci > > [39019.426650] firewire_core crc_itu_t i2c_i801 ahci sky2 libahci > > i2c_smbus lpc_ich > > [39019.426658] CPU: 6 PID: 14136 Comm: kworker/u16:0 Tainted: GW > > I 5.10.0-rc6 #1 > > [39019.426659] Hardware name: System manufacturer System Product > > Name/P6T DELUXE, BIOS 220909/21/2010 > > [39019.426662] Workqueue: events_unbound nv50_disp_atomic_commit_work > > [39019.426665] RIP: 0010:nv50_dmac_wait+0x1e1/0x230 > > [39019.426667] Code: 8d 48 04 48 89 4a 68 c7 00 00 00 00 20 49 8b 46 38 > > 41 c7 86 20 01 00 00 00 00 00 00 49 89 46 68 e8 e4 fc ff ff e9 76 fe ff > > ff <0f> 0b b8 92 ff ff ff e9 ed fe ff ff 49 8b be 80 00 00 00 e8 c7 fc > > [39019.426668] RSP: 0018:b79d028ebd48 EFLAGS: 00010282 > > [39019.426670] RAX: ff92 RBX: 000d RCX: > > > > [39019.426671] RDX: ff92 RSI: b79d028ebc88 RDI: > > b79d028ebd28 > > [39019.426671] RBP: b79d028ebd48 R08: R09: > > b79d028ebc58 > > [39019.426672] R10: 0030 R11: 11c4 R12: > > fffb > > [39019.426673] R13: a05fc1ebd368 R14: a05fc1ebd3a8 R15: > > a05fc2425000 > > [39019.426675] FS: () GS:a061f3d8() > > knlGS: > > [39019.426676] CS: 0010 DS: ES: CR0: 80050033 > > [39019.426677] CR2: 7fb2d58e CR3: 00026280a000 CR4: > > 06e0 > > [39019.426678] Call Trace: > > [39019.426685] base827c_image_set+0x2f/0x1d0 > > [39019.426687] nv50_wndw_flush_set+0x89/0x1c0 > > [39019.426688] nv50_disp_atomic_commit_tail+0x4e7/0x7e0 > > [39019.426693] process_one_work+0x1d4/0x370 > > [39019.426695] worker_thread+0x4a/0x3b0 > > [39019.426697] ? process_one_work+0x370/0x370 > > [39019.426699] kthread+0xfe/0x140 > > [39019.426701] ? kthread_park+0x90/0x90 > > [39019.426704] ret_from_fork+0x22/0x30 > > [39019.426706] ---[ end trace d512d675211c738c ]--- > > [39021.426751] [ cut here ] > > > > > > Thanks in advance, > > > > Bob > > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/edid: Fix uninitialized variable in drm_cvt_modes()
On Tue, Nov 3, 2020 at 2:47 PM Lyude Paul wrote:
>
> Sorry! Thought I had responded to this but apparently not, comments down below
>
> On Thu, 2020-10-22 at 14:04 -0400, Ilia Mirkin wrote:
> > On Thu, Oct 22, 2020 at 12:55 PM Lyude Paul wrote:
> > >
> > > Noticed this when trying to compile with -Wall on a kernel fork. We
> > > potentially
> > > don't set width here, which causes the compiler to complain about width
> > > potentially being uninitialized in drm_cvt_modes(). So, let's fix that.
> > >
> > > Signed-off-by: Lyude Paul
> > >
> > > Cc: # v5.9+
> > > Fixes: 3f649ab728cd ("treewide: Remove uninitialized_var() usage")
> > > Signed-off-by: Lyude Paul
> > > ---
> > > drivers/gpu/drm/drm_edid.c | 8 +++-
> > > 1 file changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 631125b46e04..2da158ffed8e 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -3094,6 +3094,7 @@ static int drm_cvt_modes(struct drm_connector
> > > *connector,
> > >
> > > for (i = 0; i < 4; i++) {
> > > int width, height;
> > > + u8 cvt_aspect_ratio;
> > >
> > > cvt = &(timing->data.other_data.data.cvt[i]);
> > >
> > > @@ -3101,7 +3102,8 @@ static int drm_cvt_modes(struct drm_connector
> > > *connector,
> > > continue;
> > >
> > > height = (cvt->code[0] + ((cvt->code[1] & 0xf0) << 4) +
> > > 1) *
> > > 2;
> > > - switch (cvt->code[1] & 0x0c) {
> > > + cvt_aspect_ratio = cvt->code[1] & 0x0c;
> > > + switch (cvt_aspect_ratio) {
> > > case 0x00:
> > > width = height * 4 / 3;
> > > break;
> > > @@ -3114,6 +3116,10 @@ static int drm_cvt_modes(struct drm_connector
> > > *connector,
> > > case 0x0c:
> > > width = height * 15 / 9;
> > > break;
> > > + default:
> >
> > What value would cvt->code[1] have such that this gets hit?
> >
> > Or is this a "compiler is broken, so let's add more code" situation?
> > If so, perhaps the code added could just be enough to silence the
> > compiler (unreachable, etc)?
>
> I mean, this information comes from the EDID which inherently means it's
> coming
> from an untrusted source so the value could be literally anything as long as
> the
> EDID has a valid checksum. Note (assuming I'm understanding this code
> correctly):
>
> drm_add_edid_modes() → add_cvt_modes() → drm_for_each_detailed_block() →
> do_cvt_mode() → drm_cvt_modes()
>
> So afaict this isn't a broken compiler but a legitimate uninitialized
> variable.
The value can be anything, but it has to be something. The switch is
on "unknown & 0x0c", so only 4 cases are possible, which are
enumerated in the switch.
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/edid: Fix uninitialized variable in drm_cvt_modes()
On Tue, Nov 3, 2020 at 5:15 PM Lyude Paul wrote:
>
> Noticed this when trying to compile with -Wall on a kernel fork. We
> potentially
> don't set width here, which causes the compiler to complain about width
> potentially being uninitialized in drm_cvt_modes(). So, let's fix that.
>
> Changes since v1:
> * Don't emit an error as this code isn't reachable, just mark it as such
>
> Signed-off-by: Lyude Paul
>
> Cc: # v5.9+
> Fixes: 3f649ab728cd ("treewide: Remove uninitialized_var() usage")
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/drm_edid.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 631125b46e04..0643b98c6383 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3094,6 +3094,7 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
>
> for (i = 0; i < 4; i++) {
> int width, height;
> + u8 cvt_aspect_ratio;
>
> cvt = &(timing->data.other_data.data.cvt[i]);
>
> @@ -3101,7 +3102,8 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
> continue;
>
> height = (cvt->code[0] + ((cvt->code[1] & 0xf0) << 4) + 1) *
> 2;
> - switch (cvt->code[1] & 0x0c) {
> + cvt_aspect_ratio = cvt->code[1] & 0x0c;
The temp var doesn't do anything now right? Previously you were using
it in the print, but now you can drop these two hunks, I think?
-ilia
> + switch (cvt_aspect_ratio) {
> case 0x00:
> width = height * 4 / 3;
> break;
> @@ -3114,6 +3116,8 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
> case 0x0c:
> width = height * 15 / 9;
> break;
> + default:
> + unreachable();
> }
>
> for (j = 1; j < 5; j++) {
> --
> 2.28.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Fix uninitialized variable in drm_cvt_modes()
On Thu, Nov 5, 2020 at 6:57 PM Lyude Paul wrote:
>
> Noticed this when trying to compile with -Wall on a kernel fork. We
> potentially
> don't set width here, which causes the compiler to complain about width
> potentially being uninitialized in drm_cvt_modes(). So, let's fix that.
>
> Changes since v1:
> * Don't emit an error as this code isn't reachable, just mark it as such
> Changes since v2:
> * Remove now unused variable
>
> Signed-off-by: Lyude Paul
>
> Cc: # v5.9+
> Fixes: 3f649ab728cd ("treewide: Remove uninitialized_var() usage")
> Signed-off-by: Lyude Paul
For the very little it's worth,
Reviewed-by: Ilia Mirkin
> ---
> drivers/gpu/drm/drm_edid.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 631125b46e04..b84efd538a70 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3114,6 +3114,8 @@ static int drm_cvt_modes(struct drm_connector
> *connector,
> case 0x0c:
> width = height * 15 / 9;
> break;
> + default:
> + unreachable();
> }
>
> for (j = 1; j < 5; j++) {
> --
> 2.28.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 3/3] util: Add SMPTE pattern support for C4 format
On Mon, Mar 14, 2022 at 9:07 AM Geert Uytterhoeven wrote:
>
> Hi Ilia,
>
> On Tue, Mar 8, 2022 at 8:57 AM Geert Uytterhoeven
> wrote:
> > On Mon, Mar 7, 2022 at 10:23 PM Ilia Mirkin wrote:
> > > On Mon, Mar 7, 2022 at 3:53 PM Geert Uytterhoeven
> > > wrote:
> > > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> > > > index 953bf95492ee150c..42d75d700700dc3d 100644
> > > > --- a/tests/util/pattern.c
> > > > +++ b/tests/util/pattern.c
> > > > @@ -608,6 +608,46 @@ static void fill_smpte_rgb16fp(const struct
> > > > util_rgb_info *rgb, void *mem,
> > > > static unsigned int smpte_middle[7] = { 6, 7, 4, 7, 2, 7, 0 };
> > > > static unsigned int smpte_bottom[8] = { 8, 9, 10, 7, 11, 7, 12, 7 };
> > > >
> > > > +static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned int
> > > > pixel)
> > > > +{
> > > > + if (x & 1)
> > > > + mem[x / 2] = (mem[x / 2] & 0xf0) | (pixel & 0x0f);
> > > > + else
> > > > + mem[x / 2] = (mem[x / 2] & 0x0f) | (pixel << 4);
> > > > +}
> > >
> > > The standard layout is MSB? i.e. first pixel goes in the upper bits of
> > > the first byte? It's been ages since I've dealt with C4 (or perhaps I
> > > never even touched it), but this seems a bit surprising.
> >
> > Exactly. All register documentation I've ever seen shows the MSB on
> > the left, i.e. for bytes:
> >
> > MSB LSB
> > +---+---+---+---+---+---+---+---+
> > | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
> > +---+---+---+---+---+---+---+---+
> >
> > IBM used to count bits in the reverse order, but still had MSB left:
> >
> > MSB LSB
> > +---+---+---+---+---+---+---+---+
> > | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
> > +---+---+---+---+---+---+---+---+
> >
> > If the reverse ordering of pixels is ever needed, a new fourcc code can
> > be introduced. Note that the fbdev API has support for both orderings
> > (see fb_bitfield.msb_right), but no driver ever sets msb_right = 1,
> > hence the fbdev core doesn't support it yet.
>
> Turns out I was wrong: fbdev ordering follows native ordering, and
> there's also FBINFO_FOREIGN_ENDIAN :-(
I haven't double-checked the meaning in fbdev, but ENDIAN-ness
generally refers to the layout of *bytes*, not *bits*. Although one
could also argue that it's the layout of "elements", and so in that
way, upper/lower values could be considered flipped. I've never gone
that far though.
Cheers,
-ilia
Re: [PATCH libdrm 3/3] util: Add SMPTE pattern support for C4 format
On Mon, Mar 14, 2022 at 10:06 AM Geert Uytterhoeven
wrote:
>
> Hi Ilia,
>
> On Mon, Mar 14, 2022 at 2:44 PM Ilia Mirkin wrote:
> > On Mon, Mar 14, 2022 at 9:07 AM Geert Uytterhoeven
> > wrote:
> > > On Tue, Mar 8, 2022 at 8:57 AM Geert Uytterhoeven
> > > wrote:
> > > > On Mon, Mar 7, 2022 at 10:23 PM Ilia Mirkin
> > > > wrote:
> > > > > On Mon, Mar 7, 2022 at 3:53 PM Geert Uytterhoeven
> > > > > wrote:
> > > > > > diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> > > > > > index 953bf95492ee150c..42d75d700700dc3d 100644
> > > > > > --- a/tests/util/pattern.c
> > > > > > +++ b/tests/util/pattern.c
> > > > > > @@ -608,6 +608,46 @@ static void fill_smpte_rgb16fp(const struct
> > > > > > util_rgb_info *rgb, void *mem,
> > > > > > static unsigned int smpte_middle[7] = { 6, 7, 4, 7, 2, 7, 0 };
> > > > > > static unsigned int smpte_bottom[8] = { 8, 9, 10, 7, 11, 7, 12, 7
> > > > > > };
> > > > > >
> > > > > > +static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned
> > > > > > int pixel)
> > > > > > +{
> > > > > > + if (x & 1)
> > > > > > + mem[x / 2] = (mem[x / 2] & 0xf0) | (pixel & 0x0f);
> > > > > > + else
> > > > > > + mem[x / 2] = (mem[x / 2] & 0x0f) | (pixel << 4);
> > > > > > +}
> > > > >
> > > > > The standard layout is MSB? i.e. first pixel goes in the upper bits of
> > > > > the first byte? It's been ages since I've dealt with C4 (or perhaps I
> > > > > never even touched it), but this seems a bit surprising.
> > > >
> > > > Exactly. All register documentation I've ever seen shows the MSB on
> > > > the left, i.e. for bytes:
> > > >
> > > > MSB LSB
> > > > +---+---+---+---+---+---+---+---+
> > > > | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0 |
> > > > +---+---+---+---+---+---+---+---+
> > > >
> > > > IBM used to count bits in the reverse order, but still had MSB left:
> > > >
> > > > MSB LSB
> > > > +---+---+---+---+---+---+---+---+
> > > > | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 |
> > > > +---+---+---+---+---+---+---+---+
> > > >
> > > > If the reverse ordering of pixels is ever needed, a new fourcc code can
> > > > be introduced. Note that the fbdev API has support for both orderings
> > > > (see fb_bitfield.msb_right), but no driver ever sets msb_right = 1,
> > > > hence the fbdev core doesn't support it yet.
> > >
> > > Turns out I was wrong: fbdev ordering follows native ordering, and
> > > there's also FBINFO_FOREIGN_ENDIAN :-(
> >
> > I haven't double-checked the meaning in fbdev, but ENDIAN-ness
> > generally refers to the layout of *bytes*, not *bits*. Although one
> > could also argue that it's the layout of "elements", and so in that
> > way, upper/lower values could be considered flipped. I've never gone
> > that far though.
>
> Yes, usually it refers to the ordering of bytes in a word.
> Here, it's about the ordering of sub-byte pixels in a byte.
> Note that with C2 and C4, there's a third ordering that comes into
> play.
> So we have:
> 1. Ordering of bytes in a word, for bpp > 8,
> 2. Ordering of pixels in a byte, for bpp < 8,
> 3. Ordering of bits in a pixel, for bpp > 1.
>
> 1. Is handled by DRM_FORMAT_BIG_ENDIAN.
OK. Note that DRM_FORMAT_BIG_ENDIAN flag for formats other than
RGBX and very similar formats is basically broken in drm. So ...
watch out. There are two setups supported for big-endian currently:
1. Legacy: radeon/nouveau, ignore the "little endian" comment about
formats and only supports AddFB, not AddFB2. The former only has
depth/bpp, not the actual format, anyways. This matches what current
user-space expects too. (quirk_addfb_prefer_host_byte_order = 1)
2. AddFB2 support with proper formats. Only used for vmwgfx and virgl
in practice for BE, IIRC. Only supports 32-bit 8bpc formats, and uses
some helpers to just flip around DRM_FORMAT_BIG_ENDIAN bit to an
equivalent format in the frontend api handling. This obviously won't
work for other formats, but none of the helpers are ready to receive
the BIG_ENDIAN bit.
Cheers,
-ilia
Re: [PATCH libdrm 3/3] util: Add SMPTE pattern support for C4 format
On Mon, Mar 7, 2022 at 3:53 PM Geert Uytterhoeven wrote:
> diff --git a/tests/util/pattern.c b/tests/util/pattern.c
> index 953bf95492ee150c..42d75d700700dc3d 100644
> --- a/tests/util/pattern.c
> +++ b/tests/util/pattern.c
> @@ -608,6 +608,46 @@ static void fill_smpte_rgb16fp(const struct
> util_rgb_info *rgb, void *mem,
> static unsigned int smpte_middle[7] = { 6, 7, 4, 7, 2, 7, 0 };
> static unsigned int smpte_bottom[8] = { 8, 9, 10, 7, 11, 7, 12, 7 };
>
> +static void write_pixel_4(uint8_t *mem, unsigned int x, unsigned int pixel)
> +{
> + if (x & 1)
> + mem[x / 2] = (mem[x / 2] & 0xf0) | (pixel & 0x0f);
> + else
> + mem[x / 2] = (mem[x / 2] & 0x0f) | (pixel << 4);
> +}
The standard layout is MSB? i.e. first pixel goes in the upper bits of
the first byte? It's been ages since I've dealt with C4 (or perhaps I
never even touched it), but this seems a bit surprising.
Re: [PATCH] MAINTAINERS: Add Helge as fbdev maintainer
On Mon, Jan 17, 2022 at 2:47 PM Helge Deller wrote: > > On 1/17/22 17:21, Helge Deller wrote: > > On 1/17/22 16:58, Thomas Zimmermann wrote: > >> Hi > >> > >> Am 17.01.22 um 16:42 schrieb Helge Deller: > >>> [...] > > c) reintroduce the state where fbcon is fast on fbdev. This is > > important for non-DRM machines, > > either when run on native hardware or in an emulator. > > d) not break DRM development > > > > Especially regarding c) I complained in [1] and got no feedback. I > > really would like to > > understand where the actual problems were and what's necessary to fix > > them. > > > > Helge > > > > [1] > > https://lore.kernel.org/r/feea8303-2b83-fc36-972c-4fc8ad723...@gmx.de > >> > >> Seems like few people read linux-fbdev these days. > >> I suggest to partly revert the patch to the point were performance > >> gets better again. > > Yes, *please*! > > That would solve my biggest concern. > > > > As far as I can see that's only 2 commits to be reverted: > > b3ec8cdf457e - "fbdev: Garbage collect fbdev scrolling acceleration, part 1 > > (from TODO list)" > > 39aead8373b3 - "fbcon: Disable accelerated scrolling"for-next-next > > > > I think both were not related to any 0-day bug reports (but again, I might > > be wrong). > > I did some more checking... > > Both patches are not related to DRM, since no DRM driver sets the > FBINFO_HWACCEL_COPYAREA or FBINFO_HWACCEL_FILLRECT flags. These used to be set by, at least, nouveau (which is a drm driver). And yeah, console responsiveness is _way_ worse without that. People keep pushing the messaging that it's the same speed to do it as memcpy, but that's just not the case in my experience, on a pretty bog-standard x86 desktop. The support got dumped, and it felt pretty clear from the messaging at the time, "too bad". Would love to see it come back. Cheers, -ilia
Re: [PATCH v5 2/3] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.
On Thu, Nov 4, 2021 at 4:03 PM Mark Yacoub wrote:
>
> From: Mark Yacoub
>
> [Why]
> 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> or Degamma props in the new CRTC state, allowing any invalid size to
> be passed on.
> 2. Each driver has its own LUT size, which could also be different for
> legacy users.
>
> [How]
> 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> assigned by the driver when it's initializing its color and CTM
> management.
> 2. Create drm_atomic_helper_check_crtc which is called by
> drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> they match the sizes in the new CRTC state.
> 3. As the LUT size check now happens in drm_atomic_helper_check, remove
> the lut check in intel_color.c
>
> Resolves: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> Tested on Zork (amdgpu) and Jacuzzi (mediatek), volteer (TGL)
>
> v3:
> 1. Check for lut pointer inside drm_check_lut_size.
>
> v2:
> 1. Remove the rename to a parent commit.
> 2. Create a drm drm_check_lut_size instead of intel only function.
>
> v1:
> 1. Fix typos
> 2. Remove the LUT size check from intel driver
> 3. Rename old LUT check to indicate it's a channel change
>
> Signed-off-by: Mark Yacoub
> ---
> drivers/gpu/drm/drm_atomic_helper.c| 52 ++
> drivers/gpu/drm/drm_color_mgmt.c | 19
> drivers/gpu/drm/i915/display/intel_color.c | 32 ++---
> include/drm/drm_atomic_helper.h| 1 +
> include/drm/drm_color_mgmt.h | 3 ++
> include/drm/drm_crtc.h | 11 +
> 6 files changed, 99 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index bc3487964fb5e..548e5d8221fb4 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -929,6 +929,54 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_helper_check_planes);
>
> +/**
> + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> + * @state: the driver state object
> + *
> + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> + * state holds them.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> +{
> + struct drm_crtc *crtc;
> + struct drm_crtc_state *new_crtc_state;
> + int i;
> +
> + for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> + if (!new_crtc_state->color_mgmt_changed)
> + continue;
> +
> + if (drm_check_lut_size(new_crtc_state->gamma_lut,
> + crtc->gamma_lut_size) ||
> + drm_check_lut_size(new_crtc_state->gamma_lut,
> + crtc->gamma_size)) {
> + drm_dbg_state(
> + state->dev,
> + "Invalid Gamma LUT size. Expected %u/%u, got
> %u.\n",
> + crtc->gamma_lut_size, crtc->gamma_size,
> +
> drm_color_lut_size(new_crtc_state->gamma_lut));
> + return -EINVAL;
> + }
> +
> + if (drm_check_lut_size(new_crtc_state->degamma_lut,
> + crtc->degamma_lut_size)) {
> + drm_dbg_state(
> + state->dev,
> + "Invalid Degamma LUT size. Expected %u, got
> %u.\n",
> + crtc->degamma_lut_size,
> + drm_color_lut_size(
> + new_crtc_state->degamma_lut));
> + return -EINVAL;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> +
> /**
> * drm_atomic_helper_check - validate state object
> * @dev: DRM device
> @@ -974,6 +1022,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> + ret = drm_atomic_helper_check_crtcs(state);
> + if (ret)
> + return ret;
> +
> if (state->legacy_cursor_update)
> state->async_update = !drm_atomic_helper_async_check(dev,
> state);
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 16a07f84948f3..c85094223b535 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> struct drm_mode_config *config = &dev->mode_config;
>
> if (degamma_lut_size) {
> + crtc->degamma_lut_size = degamma_lut_size;
> drm_object_attach_property(&crtc->base,
>
Re: [PATCH v8 4/4] NOTFORMERGE: drm/logicvc: Add plane colorkey support
FWIW this is something I added, hoping it was going to get used at some point, but I never followed up with support in xf86-video-nouveau for Xv. At this point, I'm not sure I ever will. I encoded the "enabled" part into the value with a high bit (1<<24) -- not sure that was such a great idea. All NVIDIA hardware supports colorkey (and not actual alpha, until the very latest GPUs - Volta/Turing families), although my implementation only covers the pre-G80 series (i.e. DX9 and earlier GPUs - pre-2008). Should a determination of usefulness be reached, it would be easy to implement on the remainder though. Cheers, -ilia On Wed, Dec 23, 2020 at 5:20 PM Simon Ser wrote: > > nouveau already has something for colorkey: > https://drmdb.emersion.fr/properties/4008636142/colorkey > > I know this is marked "not for merge", but it would be nice to discuss > with them and come up with a standardized property. > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/loongson: Add DRM Driver for Loongson 7A1000 bridge chip
On Wed, Dec 30, 2020 at 6:08 AM Chenyang Li wrote:
> + switch (format->format) {
> + case DRM_FORMAT_RGB565:
> + lcrtc->cfg_reg |= 0x3;
> + break;
> + case DRM_FORMAT_RGB888:
> + default:
> + lcrtc->cfg_reg |= 0x4;
> + break;
> + }
How does the scanout engine distinguish between RGB888 (24-bit) and
XRGB (32-bit)? You've listed both as supported below.
Cheers,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: Fix pageflipping for XOrg in Linux 5.11+
On Sat, Jan 2, 2021 at 1:35 PM Mario Kleiner wrote: > I'm less sure about nouveau. It uses modifiers, but has atomic support > only on nv50+ and that atomic support is off by default -- needs a > nouveau.nouveau_atomic=1 boot parameter to switch it on. It seems to > enable modifier support unconditionally regardless if atomic or not, > see: > https://elixir.bootlin.com/linux/v5.11-rc1/source/drivers/gpu/drm/nouveau/nouveau_display.c#L703 > > Atm. nouveau doesn't assign a new format_info though, so wouldn't > trigger this issue atm. Note that pre-nv50, no modifiers exist. Also, drm_drv_uses_atomic_modeset() doesn't care whether the client is an atomic client or not. It will return true for nv50+ no matter what. nouveau_atomic=1 affects whether atomic UAPI is exposed. Not sure if this impacts your discussion. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] PCI: add a REBAR size quirk for Sapphire RX 5600 XT Pulse.
On Tue, Jan 5, 2021 at 8:44 AM Christian König wrote: > > Otherwise the CPU can't fully access the BAR. > > Signed-off-by: Christian König > --- > drivers/pci/pci.c | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index 16216186b51c..b66e4703c214 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3577,7 +3577,14 @@ u32 pci_rebar_get_possible_sizes(struct pci_dev *pdev, > int bar) > return 0; > > pci_read_config_dword(pdev, pos + PCI_REBAR_CAP, &cap); > - return (cap & PCI_REBAR_CAP_SIZES) >> 4; > + cap = (cap & PCI_REBAR_CAP_SIZES) >> 4; > + > + /* Sapphire RX 5600 XT Pulse has an invalid cap dword for BAR 0 */ > + if (pdev->vendor == PCI_VENDOR_ID_ATI && pdev->device == 0x731f && > + bar == 0 && cap == 0x700) > + cap == 0x7f00; Perhaps you meant cap = 0x7f00? > + > + return cap; > } > EXPORT_SYMBOL(pci_rebar_get_possible_sizes); > > -- > 2.25.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] Re: Mullins support in xf86-video-amdgpu
On Mon, Aug 22, 2016 at 10:57 PM, Reid Hekman wrote:
> On 08/22/2016 08:27 PM, Michel Dänzer wrote:
>>
>> On 23/08/16 10:18 AM, Reid Hekman wrote:
>>>
>>>
>>> I was encouraged by a commit I saw today to add Sea Islands PCI ids to
>>> xf86-video-amdgpu. However I did not see MULLINS included. Are there
>>> other showstoppers preventing inclusion?
>>
>>
>> I assume the commit you're referring to was the one adding the
>> *Southern* Islands (SI) PCI IDs. Since Mullins is Sea Islands (CIK), it
>> should already be supported by the amdgpu kernel driver; its omission
>> from xf86-video-amdgpu is probably just an oversight. Can you send a
>> patch to the amd-gfx mailing list?
>>
>
> Michael, oops, yes, sorry, apparently my head was on the wrong AMD islands.
> Admittedly I'm a total newb at this, but David's script makes this seem
> fairly simple. Does the following look correct?
>
> Regards,
> Reid
>
> ---
> diff -aur a/src/amdgpu_chipinfo_gen.h b/src/amdgpu_chipinfo_gen.h
> --- a/src/amdgpu_chipinfo_gen.h 2016-08-22 21:25:49.101755291 -0500
> +++ b/src/amdgpu_chipinfo_gen.h 2016-08-22 21:41:26.758967344 -0500
> @@ -100,6 +100,22 @@
> { 0x983D, CHIP_FAMILY_KABINI },
> { 0x983E, CHIP_FAMILY_KABINI },
> { 0x983F, CHIP_FAMILY_KABINI },
> + { 0x9850, CHIP_FAMILY_MULLINS },
> + { 0x9851, CHIP_FAMILY_MULLINS },
> + { 0x9852, CHIP_FAMILY_MULLINS },
> + { 0x9853, CHIP_FAMILY_MULLINS },
> + { 0x9854, CHIP_FAMILY_MULLINS },
> + { 0x9855, CHIP_FAMILY_MULLINS },
> + { 0x9856, CHIP_FAMILY_MULLINS },
> + { 0x9857, CHIP_FAMILY_MULLINS },
> + { 0x9858, CHIP_FAMILY_MULLINS },
> + { 0x9859, CHIP_FAMILY_MULLINS },
> + { 0x985a, CHIP_FAMILY_MULLINS },
> + { 0x985b, CHIP_FAMILY_MULLINS },
> + { 0x985c, CHIP_FAMILY_MULLINS },
> + { 0x985d, CHIP_FAMILY_MULLINS },
> + { 0x985e, CHIP_FAMILY_MULLINS },
> + { 0x985f, CHIP_FAMILY_MULLINS },
> { 0x1304, CHIP_FAMILY_KAVERI },
> { 0x1305, CHIP_FAMILY_KAVERI },
> { 0x1306, CHIP_FAMILY_KAVERI },
> diff -aur a/src/amdgpu_chipset_gen.h b/src/amdgpu_chipset_gen.h
> --- a/src/amdgpu_chipset_gen.h 2016-08-22 21:25:49.101755291 -0500
> +++ b/src/amdgpu_chipset_gen.h 2016-08-22 21:41:26.758967344 -0500
> @@ -100,6 +100,22 @@
>{ PCI_CHIP_KABINI_983D, "KABINI" },
>{ PCI_CHIP_KABINI_983E, "KABINI" },
>{ PCI_CHIP_KABINI_983F, "KABINI" },
> + { PCI_CHIP_MULLINS 9850, "MULLINS" },
> + { PCI_CHIP_MULLINS 9851, "MULLINS" },
> + { PCI_CHIP_MULLINS 9852, "MULLINS" },
> + { PCI_CHIP_MULLINS 9853, "MULLINS" },
> + { PCI_CHIP_MULLINS 9854, "MULLINS" },
> + { PCI_CHIP_MULLINS 9855, "MULLINS" },
> + { PCI_CHIP_MULLINS 9856, "MULLINS" },
> + { PCI_CHIP_MULLINS 9857, "MULLINS" },
> + { PCI_CHIP_MULLINS 9858, "MULLINS" },
> + { PCI_CHIP_MULLINS 9859, "MULLINS" },
> + { PCI_CHIP_MULLINS 985a, "MULLINS" },
> + { PCI_CHIP_MULLINS 985b, "MULLINS" },
> + { PCI_CHIP_MULLINS 985c, "MULLINS" },
> + { PCI_CHIP_MULLINS 985d, "MULLINS" },
> + { PCI_CHIP_MULLINS 985e, "MULLINS" },
> + { PCI_CHIP_MULLINS 985f, "MULLINS" },
>{ PCI_CHIP_KAVERI_1304, "KAVERI" },
>{ PCI_CHIP_KAVERI_1305, "KAVERI" },
>{ PCI_CHIP_KAVERI_1306, "KAVERI" },
> diff -aur a/src/amdgpu_pci_chipset_gen.h b/src/amdgpu_pci_chipset_gen.h
> --- a/src/amdgpu_pci_chipset_gen.h 2016-08-22 21:25:49.104755254 -0500
> +++ b/src/amdgpu_pci_chipset_gen.h 2016-08-22 21:41:26.758967344 -0500
> @@ -100,6 +100,22 @@
> { PCI_CHIP_KABINI_983D, PCI_CHIP_KABINI_983D, RES_SHARED_VGA },
> { PCI_CHIP_KABINI_983E, PCI_CHIP_KABINI_983E, RES_SHARED_VGA },
> { PCI_CHIP_KABINI_983F, PCI_CHIP_KABINI_983F, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9850, PCI_CHIP_MULLINS 9850, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9851, PCI_CHIP_MULLINS 9851, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9852, PCI_CHIP_MULLINS 9852, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9853, PCI_CHIP_MULLINS 9853, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9854, PCI_CHIP_MULLINS 9854, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9855, PCI_CHIP_MULLINS 9855, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9856, PCI_CHIP_MULLINS 9856, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9857, PCI_CHIP_MULLINS 9857, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9858, PCI_CHIP_MULLINS 9858, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 9859, PCI_CHIP_MULLINS 9859, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 985a, PCI_CHIP_MULLINS 985a, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 985b, PCI_CHIP_MULLINS 985b, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 985c, PCI_CHIP_MULLINS 985c, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 985d, PCI_CHIP_MULLINS 985d, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 985e, PCI_CHIP_MULLINS 985e, RES_SHARED_VGA },
> + { PCI_CHIP_MULLINS 985f, PCI_CHIP_MULLINS 985f, RES_SHARED_VGA },
> { PCI_CHIP_KAVERI_1304, PCI_CHIP_KAVERI_1304, RES_SHARED_VGA },
> { PCI_CHIP_KAVERI_1305, PCI_CHIP_KAVERI_1305, RES_SHARED_VGA },
> { PCI_CHIP_KAVERI_1306, PCI_CHIP_KAVERI_1306, RES_SHARED_VGA },
> diff -aur a/src/amdgpu_pci_device_match_gen.h
> b/src/amdgpu_pci_device_match_gen.h
> --- a/src/a
Re: [Nouveau] [PATCH] drm/nouveau: fix duplication of nv50_head_atom struct
On Tue, May 14, 2019 at 3:57 PM Peteris Rudzusiks
wrote:
>
> On Tue, May 14, 2019 at 04:55:05PM +1000, Ben Skeggs wrote:
> > On Sun, 12 May 2019 at 04:23, Peteris Rudzusiks
> > wrote:
> > >
> > > nv50_head_atomic_duplicate_state() makes a copy of nv50_head_atom
> > > struct. This patch adds copying of struct member named "or", which
> > > previously was left uninitialized in the duplicated structure.
> > >
> > > Due to this bug, incorrect nhsync and nvsync values were sometimes used.
> > > In my particular case, that lead to a mismatch between the output
> > > resolution of the graphics device (GeForce GT 630 OEM) and the reported
> > > input signal resolution on the display. xrandr reported 1680x1050, but
> > > the display reported 1280x1024. As a result of this mismatch, the output
> > > on the display looked like it was cropped (only part of the output was
> > > actually visible on the display).
> > >
> > > git bisect pointed to commit 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
> > > SetControlOutputResource from head"), which added the member "or" to
> > > nv50_head_atom structure, but forgot to copy it in
> > > nv50_head_atomic_duplicate_state().
> > >
> > > Fixes: 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
> > > SetControlOutputResource from head")
> > > Signed-off-by: Peteris Rudzusiks
> > Oops, nice catch. Thank you for this, I've merged it in my tree and
> > will get it upstream ASAP.
> >
> > Thanks,
> > Ben.
> >
> Hi Ben,
>
> Thank you for taking the time to review and merge this patch.
>
> I'm new to the Linux kernel development process, so I am not sure what
> happens next. Does inclusion in your tree imply that this fix will end
> up in some (most likely - next) mainline kernel? Will it also be
> backported to 4.19 LTS branch?
>
> This bug affects all kernel versions starting from v4.18. Probably not
> that many systems though.
Ben submits a pull request to Dave Airlie (drm maintainer), and Dave
submits one to Linus for inclusion in the "official" upstream
repository. Dave just sent a pull request to Linus, who usually picks
these up within a few days (exceptions apply).
Once in the mainline tree, the "Fixes" tag is likely to cause it to
get picked up for stable. You can also nominate it for stable kernel
branch inclusion explicitly (there are instructions somewhere, but
basically you send an email to some list saying "please include commit
ABC in kernels XYZ").
What Ubuntu ships is, ultimately, up to Ubuntu. They will, however,
frequently follow the stable kernel branches, and listen to the list
above as well.
Hope this helps,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/nouveau/disp/nv50-: force scaler for any non-default LVDS/eDP modes
Higher layers tend to add a lot of modes not actually in the EDID, such
as the standard DMT modes. Changing this would be extremely intrusive to
everyone, so just force the scaler more often. There are no practical
cases we're aware of where a LVDS/eDP panel has multiple resolutions
exposed, and i915 already does it this way.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
Signed-off-by: Ilia Mirkin
---
Untested for now, hoping that the bugzilla filer will test it out. Seems
obvious though.
drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 134701a837c8..ef8d7a71564a 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -322,8 +322,13 @@ nv50_outp_atomic_check_view(struct drm_encoder *encoder,
switch (connector->connector_type) {
case DRM_MODE_CONNECTOR_LVDS:
case DRM_MODE_CONNECTOR_eDP:
- /* Force use of scaler for non-EDID modes. */
- if (adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
+ /* Don't force scaler for EDID modes with
+* same size as the native one (e.g. different
+* refresh rate)
+*/
+ if (adjusted_mode->hdisplay == native_mode->hdisplay &&
+ adjusted_mode->vdisplay == native_mode->vdisplay &&
+ adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
break;
mode = native_mode;
asyc->scaler.full = true;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/nouveau/disp/nv50-: fix center/aspect-corrected scaling
Previously center scaling would get scaling applied to it (when it was
only supposed to center the image), and aspect-corrected scaling did not
always correctly pick whether to reduce width or height for a particular
combination of inputs/outputs.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
Signed-off-by: Ilia Mirkin
---
Tested on a 1920x1200-native screen with a 640x480 mode (got letter
boxes on the side) and 720x400 mode (got letter boxes on top/bottom).
drivers/gpu/drm/nouveau/dispnv50/head.c | 28 +
1 file changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index ac97ebce5b35..e2207990d3cc 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -169,14 +169,34 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
*/
switch (mode) {
case DRM_MODE_SCALE_CENTER:
- asyh->view.oW = min((u16)umode->hdisplay, asyh->view.oW);
- asyh->view.oH = min((u16)umode_vdisplay, asyh->view.oH);
- /* fall-through */
+ /* NOTE: This will cause scaling when the input is
+* larger than the output.
+*/
+ asyh->view.oW = min(asyh->view.iW, asyh->view.oW);
+ asyh->view.oH = min(asyh->view.iH, asyh->view.oH);
+ break;
case DRM_MODE_SCALE_ASPECT:
- if (asyh->view.oH < asyh->view.oW) {
+ /* Determine whether the scaling should be on width or on
+* height. This is done by comparing the aspect ratios of the
+* sizes. If the output AR is larger than input AR, that means
+* we want to change the width (letterboxed on the
+* left/right), otherwise on the height (letterboxed on the
+* top/bottom).
+*
+* E.g. 4:3 (1.333) AR image displayed on a 16:10 (1.6) AR
+* screen will have letterboxes on the left/right. However a
+* 16:9 (1.777) AR image on that same screen will have
+* letterboxes on the top/bottom.
+*
+* inputAR = iW / iH; outputAR = oW / oH
+* outputAR > inputAR is equivalent to oW * iH > iW * oH
+*/
+ if (asyh->view.oW * asyh->view.iH > asyh->view.iW *
asyh->view.oH) {
+ /* Recompute output width, i.e. left/right letterbox */
u32 r = (asyh->view.iW << 19) / asyh->view.iH;
asyh->view.oW = ((asyh->view.oH * r) + (r / 2)) >> 19;
} else {
+ /* Recompute output height, i.e. top/bottom letterbox */
u32 r = (asyh->view.iH << 19) / asyh->view.iW;
asyh->view.oH = ((asyh->view.oW * r) + (r / 2)) >> 19;
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 1/2] drm/nouveau/disp/nv50-: force scaler for any non-default LVDS/eDP modes
Sigh ... looks like this doesn't actually do what we want. See the
last couple comments in:
https://bugs.freedesktop.org/show_bug.cgi?id=110660
It seems to work as expected with "mode" instead of "adjusted_mode".
Does that make sense? It kinda does based on the later code, which
copies mode into adjusted_mode...
Assuming it makes sense to use "mode", Ben, want to just do a fixup
locally, or want me to send a revert + new patch?
-ilia
On Mon, May 27, 2019 at 2:24 AM Ben Skeggs wrote:
>
> On Sun, 26 May 2019 at 08:41, Ilia Mirkin wrote:
> >
> > Higher layers tend to add a lot of modes not actually in the EDID, such
> > as the standard DMT modes. Changing this would be extremely intrusive to
> > everyone, so just force the scaler more often. There are no practical
> > cases we're aware of where a LVDS/eDP panel has multiple resolutions
> > exposed, and i915 already does it this way.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110660
> > Signed-off-by: Ilia Mirkin
> Thanks Ilia, grabbed both patches.
>
> > ---
> >
> > Untested for now, hoping that the bugzilla filer will test it out. Seems
> > obvious though.
> >
> > drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > index 134701a837c8..ef8d7a71564a 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > @@ -322,8 +322,13 @@ nv50_outp_atomic_check_view(struct drm_encoder
> > *encoder,
> > switch (connector->connector_type) {
> > case DRM_MODE_CONNECTOR_LVDS:
> > case DRM_MODE_CONNECTOR_eDP:
> > - /* Force use of scaler for non-EDID modes. */
> > - if (adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> > + /* Don't force scaler for EDID modes with
> > +* same size as the native one (e.g. different
> > +* refresh rate)
> > +*/
> > + if (adjusted_mode->hdisplay ==
> > native_mode->hdisplay &&
> > + adjusted_mode->vdisplay ==
> > native_mode->vdisplay &&
> > + adjusted_mode->type & DRM_MODE_TYPE_DRIVER)
> > break;
> > mode = native_mode;
> > asyc->scaler.full = true;
> > --
> > 2.21.0
> >
> > ___
> > Nouveau mailing list
> > nouv...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau: Don't disable polling in fallback mode
Ben - ping? Just ran into this myself on a NV42.
On Wed, Nov 14, 2018 at 11:01 AM Takashi Iwai wrote:
>
> On Fri, 14 Sep 2018 13:59:25 +0200,
> Martin Peres wrote:
> >
> > On 14/09/2018 10:28, Ben Skeggs wrote:
> > > On Wed, 12 Sep 2018 at 20:59, Takashi Iwai wrote:
> > >>
> > >> When a fan is controlled via linear fallback without cstate, we
> > >> shouldn't stop polling. Otherwise it won't be adjusted again and
> > >> keeps running at an initial crazy pace.
> > > Martin,
> > >
> > > Any thoughts on this?
> > >
> > > Ben.
> >
> > Wow, blast from the past!
> >
> > Anyway, the analysis is pretty spot on here. When using the cstate-based
> > fan speed (change the speed of the fan based on what frequency is used),
> > then polling is unnecessary and this function should only be called when
> > changing the pstate.
> >
> > However, in the absence of ANY information, we fallback to a
> > temperature-based management which requires constant polling, so the
> > patch is accurate and poll = false should only be set if we have a cstate.
> >
> > So, the patch is Reviewed-by: Martin Peres
>
> Just a gentle reminder: this patch seems forgotten for 4.20 merge.
> Could you guys pick it if it's OK?
>
>
> Thanks!
>
> Takashi
>
> >
> > >
> > >>
> > >> Fixes: 800efb4c2857 ("drm/nouveau/drm/therm/fan: add a fallback if no
> > >> fan control is specified in the vbios")
> > >> Bugzilla: https://bugzilla.suse.com/show_bug.cgi?id=1103356
> > >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107447
> >
> > I see that Thomas has been having issues with the noise level anyway. I
> > suggest he should bump the value of temp1_auto_point1_temp (see
> > https://www.kernel.org/doc/Documentation/thermal/nouveau_thermal).
> >
> > The default value is set to 90°C which is quite safe on these old GPUs
> > (NVIDIA G71 / nv49). I would say that it is safe to go up to 110°C.
> > Which should reduce the noise level.
> >
> > Another technique may be to reduce the minimum fan speed to something
> > lower than 30°C. It should increase the slope but reduce the noise level
> > at a given temperature.
> >
> > One reason why these GPUs run so hot on nouveau is the lack of power and
> > clock gating. I am sorry that I never finished to reverse engineer these...
> >
> > Anyway, thanks a lot for the patch!
> >
> > >> Reported-by: Thomas Blume
> > >> Signed-off-by: Takashi Iwai
> > >>
> > >> ---
> > >> drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c | 7 ---
> > >> 1 file changed, 4 insertions(+), 3 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> > >> b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> > >> index 3695cde669f8..07914e36939e 100644
> > >> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> > >> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c
> > >> @@ -132,11 +132,12 @@ nvkm_therm_update(struct nvkm_therm *therm, int
> > >> mode)
> > >> duty = nvkm_therm_update_linear(therm);
> > >> break;
> > >> case NVBIOS_THERM_FAN_OTHER:
> > >> - if (therm->cstate)
> > >> + if (therm->cstate) {
> > >> duty = therm->cstate;
> > >> - else
> > >> + poll = false;
> > >> + } else {
> > >> duty =
> > >> nvkm_therm_update_linear_fallback(therm);
> > >> - poll = false;
> > >> + }
> > >> break;
> > >> }
> > >> immd = false;
> > >> --
> > >> 2.18.0
> > >>
> > >> ___
> > >> Nouveau mailing list
> > >> nouv...@lists.freedesktop.org
> > >> https://lists.freedesktop.org/mailman/listinfo/nouveau
> >
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Nouveau module results in total lockups without any dmesg trace on a NP900X5N Kaby Lake machine
On Tue, Jan 1, 2019 at 4:06 PM Jan Vlietland wrote: > > Hi Ben, David and Daniel , > > First of all happy new year. Based on advice of Greg K-H herewith a mail > about a number of Nouveau issues with my laptop. > > I installed various Kali linux versions up to Linux 4.20.0-rc7 > (downloaded, compiled and installed) on a Samsung NP900X5N laptop and > have an issue with the driver after loading. > > My configuration: > > - i7 7500 > - 16 gb / 256 gb ssd > - nvidia 940MX (for 3D graphics) > > When I enable loading of the nouveau module for my Nvidia 3D card I get > three MMIO faults: > > [ 35.984104] nouveau :01:00.0: bus: MMIO read of FAULT at > 6013d4 [ IBUS ] > [ 35.997510] nouveau :01:00.0: bus: MMIO read of FAULT at > 10ac08 [ IBUS ] > [ 37.551790] nouveau :01:00.0: bus: MMIO read of FAULT at > 619444 [ IBUS ] > > I see currenty varous discussions on bugzilla: (as summarized by Bruno > Pagani) https://bugs.freedesktop.org/show_bug.cgi?id=100423. > > But I do not see any confirmed solutions on the MMIO faults. > > The module is loaded but X server cannot run in framebuffer mode. I > assume that the module does not provide any video memory to X to run in > graphics mode. > > First of all I would like to understand what the faults impose. > And I also would like to help you providing testing to fix the errors. The faults are, generally, nothing to worry about, esp if they occur infrequently. It's one bit or another of code that's poking at a part of the GPU it shouldn't be touching. To the best of my knowledge, 940MX (GM108) should work reasonably well. Perhaps it would make most sense if you posted about some of your other issues (usually GM108's are have no outputs, so only usable as offload devices). Feel free to join #nouveau on irc.freenode.net to get more info as well. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Nouveau module X server not starting on a NP900X5N Kaby Lake machine
On Tue, Jan 1, 2019 at 5:30 PM Jan Vlietland wrote: > > Hi Ilia, many thanks for answering my mail. > > Tonight I tried to see what happens if I generate a xorg.conf file and place > it in /etc/X11/xorg.conf, as described here: > https://askubuntu.com/questions/4662/where-is-the-x-org-config-file-how-do-i-configure-x-there > > When I do that X starts without the framebuffer error. X starts with a > backtrace list in the shell and then stops with the error: > > 'Segmentation fault at address 0x0. > > Fatal sever error etc etc etc. Unless you're an advanced user, you'll get the best results by not supplying a manual xorg.conf. Generically, this indicates that you messed something up. Without knowing precisely what the contents of that file are, it would be difficult to say what exactly went wrong. However I wouldn't advise this path without a good reason. > > Hope this helps! > > In fact the above is part of a much bigger issue I have with the > machine. When I enable the i915 module (Kaby lake native video) my > screen goed black after a while. The machine is totally stuck in that > state. Even ssh connection is not possible. It shows no errors in the > (saved) logs after restarting the machine. > > So I disabled the i915 module and try to get the nvidia card running. > Without any luck. > > Thank you for inviting me for irc.freenode.net. What it the procss to > get access? It's an IRC network like any other. More info about the network available at https://freenode.net/ It's open to anyone... #nouveau for nouveau, #intel-gfx for intel. > > I have included the full dmesg in zip format. > > For me it is a showstopper using the machine with Linux. I really do not > understand that I am the only person on this planet that cannot run > Linux on a plain vanilla Kaby lake machine. I don't know the specifics of your laptop, but on many other GM108M laptops, the displays are only attached to the Intel GPU. So running without i915 is just not an option, if you want anything displayed. You would be able to use the GM108M chip for 3D acceleration if you chose, but nothing to do with actual display. If your screen goes black with i915 loaded, I suspect that you'd be better served reporting this issue to Intel. From your logs, you also appear to have a variety of combinations of nomodeset/.modeset=0 combinations -- these will just impede the proper mode of operation. The i915 and nouveau drivers effectively do nothing under those conditions. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
Hi Gerd, What happened with this series (and the next one)? Semi-relatedly, I wonder if it wouldn't be better to just dump the BIG_ENDIAN flag and just define the "host" format to be the right one for a consistent little-endian interpretation. Cheers, -ilia On Wed, Sep 5, 2018 at 2:04 AM Gerd Hoffmann wrote: > > Patch series adds some convinience #defines for host byteoder drm > formats. It fixes drm_mode_addfb() behavior on bigendian machines. For > bug compatibility reasons a mode_config quirk activates the fix. > bochs and virtio-gpu drivers are updated to use the new #defines, set > the new driver feature flag and fix some issues. > > Gerd Hoffmann (6): > drm: replace DRIVER_PREFER_XBGR_30BPP driver flag with mode_config > quirk > drm: byteorder: add DRM_FORMAT_HOST_* > drm: do not mask out DRM_FORMAT_BIG_ENDIAN > drm: fix drm_mode_addfb() on big endian machines. > drm/bochs: fix DRM_FORMAT_* handling for big endian machines. > drm/virtio: fix DRM_FORMAT_* handling > > include/drm/drm_drv.h| 1 - > include/drm/drm_fourcc.h | 22 + > include/drm/drm_mode_config.h| 15 + > drivers/gpu/drm/bochs/bochs_fbdev.c | 5 ++- > drivers/gpu/drm/bochs/bochs_kms.c| 34 +++- > drivers/gpu/drm/bochs/bochs_mm.c | 2 +- > drivers/gpu/drm/drm_framebuffer.c| 17 -- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_display.c | 5 +++ > drivers/gpu/drm/virtio/virtgpu_fb.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_gem.c | 7 +++-- > drivers/gpu/drm/virtio/virtgpu_plane.c | 54 > ++-- > 12 files changed, 101 insertions(+), 65 deletions(-) > > -- > 2.9.3 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/6] drm: byteorder fixes
On Fri, Jan 11, 2019, 05:26 Daniel Vetter On Fri, Jan 11, 2019 at 10:43:42AM +0100, Gerd Hoffmann wrote: > > On Fri, Jan 11, 2019 at 10:11:09AM +0100, Daniel Vetter wrote: > > > On Fri, Jan 11, 2019 at 10:08 AM Gerd Hoffmann > wrote: > > > > > > > > On Fri, Jan 11, 2019 at 01:17:47AM -0500, Ilia Mirkin wrote: > > > > > Hi Gerd, > > > > > > > > > > What happened with this series (and the next one)? > > > > > > > > Landed upstream in 4.20. > Huh. I must have been looking at an option tree. > > > > > > > > Semi-relatedly, I wonder if it wouldn't be better to just dump the > > > > > BIG_ENDIAN flag and just define the "host" format to be the right > one > > > > > for a consistent little-endian interpretation. > > > > > > > > Not fully sure what you mean here. > > > > > > > > The big endian flag is needed to specify RGB565 or XRGB1555 format in > > > > big endian byte order. For 32 bit depth we don't need it because > > > > XRGB big endian == BGRX little endian (see also > > > > include/drm/drm_fourcc.h). > Bleargh. I was thinking xrgb1555 le == bgrx5551 be, but that's clearly false. Made sense to me last night though. That makes my suggestion considerably less appealing. > > > > > Yeah we'd need to add a few more fourcc codes for the missing > > > big-endian versions. > > > > If we do that then yes we can drop the BIG_ENDIAN flag. > > > > Not sure what the userspace API implications are, > > ADDFB2 ioctl comes to mind. > > > > > Aside from that I like Ilia's idea of removing > > > the BIG_ENDIAN flag, it causes lots of aliasing among formats, doesn't > > > change anything with others, and we probably have a huge mess already > > > ... > > > > Shouldn't be too messy. The place where the DRM_FORMAT_HOST_* formats > > are defined (include/drm/drm_fourcc.h) is almost the only place where > > DRM_FORMAT_BIG_ENDIAN is used, and for the DRM_FORMAT_HOST_* users the > > switch should be transparent. > > Yeah we'd need to review current userspace, but I suspect the overlap of > "uses addfb2" and "runs on BE platform" is 0. > Most importantly, "uses bigendian flag" is 0 since we would previously error out on such modesets. Although the need to double up on the 16bpp formats makes this not really worth it. Ohhh well. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/11] drm/modes: Add DRM_SIMPLE_MODE()
You don't appear to set the mm in the new macro. Not sure if it's on
purpose.
On Sun, Jan 20, 2019, 06:43 Noralf Trønnes This adds a helper macro to specify modes that only contain info about
> resolution.
>
> Signed-off-by: Noralf Trønnes
> ---
> drivers/gpu/drm/tinydrm/hx8357d.c | 2 +-
> drivers/gpu/drm/tinydrm/ili9225.c | 2 +-
> drivers/gpu/drm/tinydrm/ili9341.c | 2 +-
> drivers/gpu/drm/tinydrm/mi0283qt.c | 2 +-
> drivers/gpu/drm/tinydrm/repaper.c | 8
> drivers/gpu/drm/tinydrm/st7586.c | 2 +-
> drivers/gpu/drm/tinydrm/st7735r.c | 2 +-
> include/drm/drm_modes.h| 16
> include/drm/tinydrm/tinydrm.h | 23 ---
> 9 files changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/tinydrm/hx8357d.c
> b/drivers/gpu/drm/tinydrm/hx8357d.c
> index 8bbd0beafc6a..5a1ec0451c19 100644
> --- a/drivers/gpu/drm/tinydrm/hx8357d.c
> +++ b/drivers/gpu/drm/tinydrm/hx8357d.c
> @@ -181,7 +181,7 @@ static const struct drm_simple_display_pipe_funcs
> hx8357d_pipe_funcs = {
> };
>
> static const struct drm_display_mode yx350hv15_mode = {
> - TINYDRM_MODE(320, 480, 60, 75),
> + DRM_SIMPLE_MODE(320, 480, 60, 75),
> };
>
> DEFINE_DRM_GEM_CMA_FOPS(hx8357d_fops);
> diff --git a/drivers/gpu/drm/tinydrm/ili9225.c
> b/drivers/gpu/drm/tinydrm/ili9225.c
> index 43a3b68d90a2..d40814d370e2 100644
> --- a/drivers/gpu/drm/tinydrm/ili9225.c
> +++ b/drivers/gpu/drm/tinydrm/ili9225.c
> @@ -332,7 +332,7 @@ static const struct drm_simple_display_pipe_funcs
> ili9225_pipe_funcs = {
> };
>
> static const struct drm_display_mode ili9225_mode = {
> - TINYDRM_MODE(176, 220, 35, 44),
> + DRM_SIMPLE_MODE(176, 220, 35, 44),
> };
>
> DEFINE_DRM_GEM_CMA_FOPS(ili9225_fops);
> diff --git a/drivers/gpu/drm/tinydrm/ili9341.c
> b/drivers/gpu/drm/tinydrm/ili9341.c
> index 713bb2dd7e04..063f4f07f811 100644
> --- a/drivers/gpu/drm/tinydrm/ili9341.c
> +++ b/drivers/gpu/drm/tinydrm/ili9341.c
> @@ -137,7 +137,7 @@ static const struct drm_simple_display_pipe_funcs
> ili9341_pipe_funcs = {
> };
>
> static const struct drm_display_mode yx240qv29_mode = {
> - TINYDRM_MODE(240, 320, 37, 49),
> + DRM_SIMPLE_MODE(240, 320, 37, 49),
> };
>
> DEFINE_DRM_GEM_CMA_FOPS(ili9341_fops);
> diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c
> b/drivers/gpu/drm/tinydrm/mi0283qt.c
> index 82a92ec9ae3c..3d067c2ba1bc 100644
> --- a/drivers/gpu/drm/tinydrm/mi0283qt.c
> +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c
> @@ -145,7 +145,7 @@ static const struct drm_simple_display_pipe_funcs
> mi0283qt_pipe_funcs = {
> };
>
> static const struct drm_display_mode mi0283qt_mode = {
> - TINYDRM_MODE(320, 240, 58, 43),
> + DRM_SIMPLE_MODE(320, 240, 58, 43),
> };
>
> DEFINE_DRM_GEM_CMA_FOPS(mi0283qt_fops);
> diff --git a/drivers/gpu/drm/tinydrm/repaper.c
> b/drivers/gpu/drm/tinydrm/repaper.c
> index b037c6540cf3..72d30151ecd8 100644
> --- a/drivers/gpu/drm/tinydrm/repaper.c
> +++ b/drivers/gpu/drm/tinydrm/repaper.c
> @@ -860,28 +860,28 @@ static const uint32_t repaper_formats[] = {
> };
>
> static const struct drm_display_mode repaper_e1144cs021_mode = {
> - TINYDRM_MODE(128, 96, 29, 22),
> + DRM_SIMPLE_MODE(128, 96, 29, 22),
> };
>
> static const u8 repaper_e1144cs021_cs[] = { 0x00, 0x00, 0x00, 0x00,
> 0x00, 0x0f, 0xff, 0x00 };
>
> static const struct drm_display_mode repaper_e1190cs021_mode = {
> - TINYDRM_MODE(144, 128, 36, 32),
> + DRM_SIMPLE_MODE(144, 128, 36, 32),
> };
>
> static const u8 repaper_e1190cs021_cs[] = { 0x00, 0x00, 0x00, 0x03,
> 0xfc, 0x00, 0x00, 0xff };
>
> static const struct drm_display_mode repaper_e2200cs021_mode = {
> - TINYDRM_MODE(200, 96, 46, 22),
> + DRM_SIMPLE_MODE(200, 96, 46, 22),
> };
>
> static const u8 repaper_e2200cs021_cs[] = { 0x00, 0x00, 0x00, 0x00,
> 0x01, 0xff, 0xe0, 0x00 };
>
> static const struct drm_display_mode repaper_e2271cs021_mode = {
> - TINYDRM_MODE(264, 176, 57, 38),
> + DRM_SIMPLE_MODE(264, 176, 57, 38),
> };
>
> static const u8 repaper_e2271cs021_cs[] = { 0x00, 0x00, 0x00, 0x7f,
> diff --git a/drivers/gpu/drm/tinydrm/st7586.c
> b/drivers/gpu/drm/tinydrm/st7586.c
> index 01a8077954b3..5ee7db561349 100644
> --- a/drivers/gpu/drm/tinydrm/st7586.c
> +++ b/drivers/gpu/drm/tinydrm/st7586.c
> @@ -312,7 +312,7 @@ static const struct drm_simple_display_pipe_funcs
> st7586_pipe_funcs = {
> };
>
> static const struct drm_display_mode st7586_mode = {
> - TINYDRM_MODE(178, 128, 37, 27),
> + DRM_SIMPLE_MODE(178, 128, 37, 27),
> };
>
> DEFINE_DRM_GEM_CMA_FOPS(st7586_fops);
> diff --git a/drivers/gpu/drm/tinydrm/st7735r.c
> b/drivers/gpu/drm/tinydrm/st7735r.c
> index 3bab9a9569a6..6c7904c205f0 100644
> --- a/drivers/gpu/drm/tinydrm/st7735r.c
> +++ b/drivers/gpu/drm/tinydrm/st7735r.c
> @@ -111,7 +111,7
Re: drm/nouveau/bios/ramcfg, setting of RON pull value
On Sat, Feb 16, 2019 at 10:02 AM Colin Ian King
wrote:
>
> Hi,
>
> Static Analysis with CoverityScan as detected an issue with the setting
> of the RON pull value in function nvkm_gddr3_calc in
> drm/nouveau/bios/ramcfg.c
>
> This was introduced by commit: c25bf7b6155cb ("drm/nouveau/bios/ramcfg:
> Separate out RON pull value")
>
> CoverityScan reports the issue as follows:
>
> 84case 0x20:
> 85CWL = (ram->next->bios.timing[1] & 0x0f80) >> 7;
> 86CL = (ram->next->bios.timing[1] & 0x001f) >> 0;
> 87WR = (ram->next->bios.timing[2] & 0x007f) >> 16;
> 88/* XXX: Get these values from the VBIOS instead */
> 89DLL = !(ram->mr[1] & 0x1);
>
>CID 1324005 (#1 of 1): Operands don't affect result
> (CONSTANT_EXPRESSION_RESULT)
>
> result_independent_of_operands: !(ram->mr[1] & 768) >> 8 is 0 regardless
> of the values of its operands. This occurs as the operand of assignment.
>
> 90RON = !(ram->mr[1] & 0x300) >> 8;
> 91break;
>
> Looking at this, I believe perhaps the correct setting could be:
>
> RON = !((ram->mr[1] & 0x300) >> 8);
>
> ..however I don't have the datasheet available for the H/W so I'm not
> sure if this a correct fix.
Actually looking at the code a bit, I suspect it should just be
RON = (ram->mr[1] & 0x300) >> 8;
since later on, when we recompose the MR (memory register) value, we do:
ram->mr[1] |= (RON & 0x03) << 8;
(And the whole point here is that we don't know how to get the proper
RON value for that timing table version, so we just copy whatever used
to be there in that case.)
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 00/10] Add C8, 30bpp and FP16 support to modetest
This series improves the pattern generation logic to support additional formats, as well as a new "gradient" pattern (see patch comments on why I found it useful). Furthermore, these formats are piped through to modetest, including the ability to set a gamma table, which is necessary for the C8 indexed format. This was tested on nouveau, and used for bring-up of the C8, XB30, and FP16 formats on the NVIDIA hardware that supports these. Ilia Mirkin (10): util: add C8 format, support it with SMPTE pattern util: fix MAKE_RGBA macro for 10bpp modes util: add gradient pattern util: add fp16 format support util: add cairo drawing for 30bpp formats when available modetest: don't pretend that atomic mode includes a format modetest: add an add_property_optional variant that does not print errors modetest: add C8 support to generate SMPTE pattern modetest: add the ability to specify fill patterns on the commandline modetest: add FP16 format support tests/modetest/buffers.c | 13 ++ tests/modetest/modetest.c | 109 -- tests/util/format.c | 7 + tests/util/pattern.c | 432 +- tests/util/pattern.h | 7 + 5 files changed, 543 insertions(+), 25 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 02/10] util: fix MAKE_RGBA macro for 10bpp modes
We need to shift the values up, otherwise we'd end up with a negative
shift. This works for up-to 16-bit components, which is fine for now.
Signed-off-by: Ilia Mirkin
---
tests/util/pattern.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index c84fee5a..8bdebd2c 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -60,11 +60,22 @@ struct color_yuv {
.u = MAKE_YUV_601_U(r, g, b), \
.v = MAKE_YUV_601_V(r, g, b) }
+static inline uint32_t shiftcolor(const struct util_color_component *comp,
+ uint32_t value)
+{
+ /* Fill the low bits with the high bits. */
+ value = (value << 8) | value;
+ /* Shift down to remove unwanted low bits */
+ value = value >> (16 - comp->length);
+ /* Shift back up to where the value should be */
+ return value << comp->offset;
+}
+
#define MAKE_RGBA(rgb, r, g, b, a) \
- r) >> (8 - (rgb)->red.length)) << (rgb)->red.offset) | \
-(((g) >> (8 - (rgb)->green.length)) << (rgb)->green.offset) | \
-(((b) >> (8 - (rgb)->blue.length)) << (rgb)->blue.offset) | \
-(((a) >> (8 - (rgb)->alpha.length)) << (rgb)->alpha.offset))
+ (shiftcolor(&(rgb)->red, (r)) | \
+shiftcolor(&(rgb)->green, (g)) | \
+shiftcolor(&(rgb)->blue, (b)) | \
+shiftcolor(&(rgb)->alpha, (a)))
#define MAKE_RGB24(rgb, r, g, b) \
{ .value = MAKE_RGBA(rgb, r, g, b, 0) }
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 08/10] modetest: add C8 support to generate SMPTE pattern
This includes logic to configure the LUT accordingly.
Signed-off-by: Ilia Mirkin
---
tests/modetest/buffers.c | 2 ++
tests/modetest/modetest.c | 47 ++-
2 files changed, 44 insertions(+), 5 deletions(-)
diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c
index 9b635c0c..5ec4ec8e 100644
--- a/tests/modetest/buffers.c
+++ b/tests/modetest/buffers.c
@@ -135,6 +135,7 @@ bo_create(int fd, unsigned int format,
int ret;
switch (format) {
+ case DRM_FORMAT_C8:
case DRM_FORMAT_NV12:
case DRM_FORMAT_NV21:
case DRM_FORMAT_NV16:
@@ -275,6 +276,7 @@ bo_create(int fd, unsigned int format,
planes[2] = virtual + offsets[2];
break;
+ case DRM_FORMAT_C8:
case DRM_FORMAT_ARGB:
case DRM_FORMAT_XRGB:
case DRM_FORMAT_ABGR:
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 71ddc861..7bb21d17 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1089,6 +1089,42 @@ static bool add_property_optional(struct device *dev,
uint32_t obj_id,
return set_property(dev, &p);
}
+static void set_gamma(struct device *dev, unsigned crtc_id, unsigned fourcc)
+{
+ unsigned blob_id = 0;
+ /* TODO: support 1024-sized LUTs, when the use-case arises */
+ struct drm_color_lut gamma_lut[256];
+ int i, ret;
+
+ if (fourcc == DRM_FORMAT_C8) {
+ /* TODO: Add C8 support for more patterns */
+ util_smpte_c8_gamma(256, gamma_lut);
+ drmModeCreatePropertyBlob(dev->fd, gamma_lut,
sizeof(gamma_lut), &blob_id);
+ } else {
+ for (i = 0; i < 256; i++) {
+ gamma_lut[i].red =
+ gamma_lut[i].green =
+ gamma_lut[i].blue = i << 8;
+ }
+ }
+
+ add_property_optional(dev, crtc_id, "DEGAMMA_LUT", 0);
+ add_property_optional(dev, crtc_id, "CTM", 0);
+ if (!add_property_optional(dev, crtc_id, "GAMMA_LUT", blob_id)) {
+ uint16_t r[256], g[256], b[256];
+
+ for (i = 0; i < 256; i++) {
+ r[i] = gamma_lut[i].red;
+ g[i] = gamma_lut[i].green;
+ b[i] = gamma_lut[i].blue;
+ }
+
+ ret = drmModeCrtcSetGamma(dev->fd, crtc_id, 256, r, g, b);
+ if (ret)
+ fprintf(stderr, "failed to set gamma: %s\n",
strerror(errno));
+ }
+}
+
static int atomic_set_plane(struct device *dev, struct plane_arg *p,
int pattern, bool
update)
{
@@ -1270,6 +1306,8 @@ static void atomic_set_planes(struct device *dev, struct
plane_arg *p,
for (i = 0; i < count; i++) {
if (i > 0)
pattern = UTIL_PATTERN_TILES;
+ else
+ set_gamma(dev, p[i].crtc_id, p[i].fourcc);
if (atomic_set_plane(dev, &p[i], pattern, update))
return;
@@ -1454,6 +1492,8 @@ static void set_mode(struct device *dev, struct pipe_arg
*pipes, unsigned int co
fprintf(stderr, "failed to set mode: %s\n",
strerror(errno));
return;
}
+
+ set_gamma(dev, pipe->crtc->crtc->crtc_id, pipe->fourcc);
}
}
@@ -1728,11 +1768,8 @@ static int parse_plane(struct plane_arg *plane, const
char *p)
}
if (*end == '@') {
- p = end + 1;
- if (strlen(p) != 4)
- return -EINVAL;
-
- strcpy(plane->format_str, p);
+ strncpy(plane->format_str, end + 1, 4);
+ plane->format_str[4] = '\0';
} else {
strcpy(plane->format_str, "XR24");
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 01/10] util: add C8 format, support it with SMPTE pattern
This also adds a helper to generate a color LUT, which has to be used in
conjunction with the C8 indexed format.
Signed-off-by: Ilia Mirkin
---
tests/util/format.c | 2 ++
tests/util/pattern.c | 75
tests/util/pattern.h | 4 +++
3 files changed, 81 insertions(+)
diff --git a/tests/util/format.c b/tests/util/format.c
index 15ac5e1e..e52213bb 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -39,6 +39,8 @@
.yuv = { (order), (xsub), (ysub), (chroma_stride) }
static const struct util_format_info format_info[] = {
+ /* Indexed */
+ { DRM_FORMAT_C8, "C8" },
/* YUV packed */
{ DRM_FORMAT_UYVY, "UYVY", MAKE_YUV_INFO(YUV_YCbCr | YUV_CY, 2, 2, 2) },
{ DRM_FORMAT_VYUY, "VYUY", MAKE_YUV_INFO(YUV_YCrCb | YUV_CY, 2, 2, 2) },
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 9fa0a417..c84fee5a 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -457,6 +457,79 @@ static void fill_smpte_rgb32(const struct util_rgb_info
*rgb, void *mem,
}
}
+static void fill_smpte_c8(void *mem, unsigned int width, unsigned int height,
+ unsigned int stride)
+{
+ unsigned int x;
+ unsigned int y;
+
+ for (y = 0; y < height * 6 / 9; ++y) {
+ for (x = 0; x < width; ++x)
+ ((uint8_t *)mem)[x] = x * 7 / width;
+ mem += stride;
+ }
+
+ for (; y < height * 7 / 9; ++y) {
+ for (x = 0; x < width; ++x)
+ ((uint8_t *)mem)[x] = 7 + (x * 7 / width);
+ mem += stride;
+ }
+
+ for (; y < height; ++y) {
+ for (x = 0; x < width * 5 / 7; ++x)
+ ((uint8_t *)mem)[x] =
+ 14 + (x * 4 / (width * 5 / 7));
+ for (; x < width * 6 / 7; ++x)
+ ((uint8_t *)mem)[x] =
+ 14 + ((x - width * 5 / 7) * 3
+ / (width / 7) + 4);
+ for (; x < width; ++x)
+ ((uint8_t *)mem)[x] = 14 + 7;
+ mem += stride;
+ }
+}
+
+void util_smpte_c8_gamma(unsigned size, struct drm_color_lut *lut)
+{
+ if (size < 7 + 7 + 8) {
+ printf("Error: gamma too small: %d < %d\n", size, 7 + 7 + 8);
+ return;
+ }
+ memset(lut, size * sizeof(struct drm_color_lut), 0);
+
+#define FILL_COLOR(idx, r, g, b) \
+ lut[idx].red = (r) << 8; \
+ lut[idx].green = (g) << 8; \
+ lut[idx].blue = (b) << 8
+
+ FILL_COLOR( 0, 192, 192, 192); /* grey */
+ FILL_COLOR( 1, 192, 192, 0 ); /* yellow */
+ FILL_COLOR( 2, 0, 192, 192); /* cyan */
+ FILL_COLOR( 3, 0, 192, 0 ); /* green */
+ FILL_COLOR( 4, 192, 0, 192); /* magenta */
+ FILL_COLOR( 5, 192, 0, 0 ); /* red */
+ FILL_COLOR( 6, 0, 0, 192); /* blue */
+
+ FILL_COLOR( 7, 0, 0, 192); /* blue */
+ FILL_COLOR( 8, 19, 19, 19 ); /* black */
+ FILL_COLOR( 9, 192, 0, 192); /* magenta */
+ FILL_COLOR(10, 19, 19, 19 ); /* black */
+ FILL_COLOR(11, 0, 192, 192); /* cyan */
+ FILL_COLOR(12, 19, 19, 19 ); /* black */
+ FILL_COLOR(13, 192, 192, 192); /* grey */
+
+ FILL_COLOR(14, 0, 33, 76); /* in-phase */
+ FILL_COLOR(15, 255, 255, 255); /* super white */
+ FILL_COLOR(16, 50, 0, 106); /* quadrature */
+ FILL_COLOR(17, 19, 19, 19); /* black */
+ FILL_COLOR(18, 9, 9, 9);/* 3.5% */
+ FILL_COLOR(19, 19, 19, 19); /* 7.5% */
+ FILL_COLOR(20, 29, 29, 29); /* 11.5% */
+ FILL_COLOR(21, 19, 19, 19); /* black */
+
+#undef FILL_COLOR
+}
+
static void fill_smpte(const struct util_format_info *info, void *planes[3],
unsigned int width, unsigned int height,
unsigned int stride)
@@ -464,6 +537,8 @@ static void fill_smpte(const struct util_format_info *info,
void *planes[3],
unsigned char *u, *v;
switch (info->format) {
+ case DRM_FORMAT_C8:
+ return fill_smpte_c8(planes[0], width, height, stride);
case DRM_FORMAT_UYVY:
case DRM_FORMAT_VYUY:
case DRM_FORMAT_YUYV:
diff --git a/tests/util/pattern.h b/tests/util/pattern.h
index d5c4260c..c8708d02 100644
--- a/tests/util/pattern.h
+++ b/tests/util/pattern.h
@@ -26,6 +26,8 @@
#ifndef UTIL_PATTERN_H
#define UTIL_PATTERN_H
+#include
+
enum util_fill_pattern {
UTIL_PATTERN_TILES,
UTIL_PATTERN_PLAIN,
@@ -36,4 +38,6 @@ void util_fill_pattern(uint32_t format, enum
util_fill_pattern pattern,
void *planes[3], unsigned int width,
unsigned int height, unsigned int stride);
+void util_smpte_c8
[PATCH libdrm 03/10] util: add gradient pattern
The idea is to have a horizontal pattern split into two with the top and
bottom halves having different precision. This allows one to see whether
10bpc support is working properly or not, as there are many pieces to
the puzzle beyond the basic format support (gamma ramps, bpc encodings,
etc).
This is really only useful on 10bpc formats, but we also add support for
8bpc formats to ease testing. In the future, this could be applied to
16bpc formats as well.
Signed-off-by: Ilia Mirkin
---
tests/util/pattern.c | 113 +--
tests/util/pattern.h | 1 +
2 files changed, 109 insertions(+), 5 deletions(-)
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 8bdebd2c..ff110fd7 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -60,9 +60,11 @@ struct color_yuv {
.u = MAKE_YUV_601_U(r, g, b), \
.v = MAKE_YUV_601_V(r, g, b) }
-static inline uint32_t shiftcolor(const struct util_color_component *comp,
+/* This function takes 8-bit color values */
+static inline uint32_t shiftcolor8(const struct util_color_component *comp,
uint32_t value)
{
+ value &= 0xff;
/* Fill the low bits with the high bits. */
value = (value << 8) | value;
/* Shift down to remove unwanted low bits */
@@ -71,11 +73,30 @@ static inline uint32_t shiftcolor(const struct
util_color_component *comp,
return value << comp->offset;
}
+/* This function takes 10-bit color values */
+static inline uint32_t shiftcolor10(const struct util_color_component *comp,
+ uint32_t value)
+{
+ value &= 0x3ff;
+ /* Fill the low bits with the high bits. */
+ value = (value << 6) | (value >> 4);
+ /* Shift down to remove unwanted low bits */
+ value = value >> (16 - comp->length);
+ /* Shift back up to where the value should be */
+ return value << comp->offset;
+}
+
+#define MAKE_RGBA10(rgb, r, g, b, a) \
+ (shiftcolor10(&(rgb)->red, (r)) | \
+shiftcolor10(&(rgb)->green, (g)) | \
+shiftcolor10(&(rgb)->blue, (b)) | \
+shiftcolor10(&(rgb)->alpha, (a)))
+
#define MAKE_RGBA(rgb, r, g, b, a) \
- (shiftcolor(&(rgb)->red, (r)) | \
-shiftcolor(&(rgb)->green, (g)) | \
-shiftcolor(&(rgb)->blue, (b)) | \
-shiftcolor(&(rgb)->alpha, (a)))
+ (shiftcolor8(&(rgb)->red, (r)) | \
+shiftcolor8(&(rgb)->green, (g)) | \
+shiftcolor8(&(rgb)->blue, (b)) | \
+shiftcolor8(&(rgb)->alpha, (a)))
#define MAKE_RGB24(rgb, r, g, b) \
{ .value = MAKE_RGBA(rgb, r, g, b, 0) }
@@ -912,6 +933,85 @@ static void fill_plain(void *planes[3],
memset(planes[0], 0x77, stride * height);
}
+static void fill_gradient_rgb32(const struct util_rgb_info *rgb,
+ void *mem,
+ unsigned int width, unsigned int height,
+ unsigned int stride)
+{
+ int i, j;
+
+ for (i = 0; i < height / 2; i++) {
+ uint32_t *row = mem;
+
+ for (j = 0; j < width / 2; j++) {
+ uint32_t value = MAKE_RGBA10(rgb, j & 0x3ff, j & 0x3ff,
j & 0x3ff, 0);
+ row[2*j] = row[2*j+1] = value;
+ }
+ mem += stride;
+ }
+
+ for (; i < height; i++) {
+ uint32_t *row = mem;
+
+ for (j = 0; j < width / 2; j++) {
+ uint32_t value = MAKE_RGBA10(rgb, j & 0x3fc, j & 0x3fc,
j & 0x3fc, 0);
+ row[2*j] = row[2*j+1] = value;
+ }
+ mem += stride;
+ }
+}
+
+/* The gradient pattern creates two horizontal gray gradients, split
+ * into two halves. The top half has 10bpc precision, the bottom half
+ * has 8bpc precision. When using with a 10bpc fb format, there are 3
+ * possible outcomes:
+ *
+ * - Pixel data is encoded as 8bpc to the display, no dithering. This
+ *would lead to the top and bottom halves looking identical.
+ *
+ * - Pixel data is encoded as 8bpc to the display, with dithering. This
+ *would lead to there being a visible difference between the two halves,
+ *but the top half would look a little speck-y due to the dithering.
+ *
+ * - Pixel data is encoded at 10bpc+ to the display (which implies
+ *the display is able to show this level of depth). This should
+ *lead to the top half being a very clean gradient, and visibly different
+ *from the bottom half.
+ *
+ * Once we support additional fb formats, this approach could be extended
+ * to distinguish even higher bpc precisions.
+ *
+ * Note that due to practical size considerations, for the screens
+ * where this matters, the pattern actually emits str
[PATCH libdrm 09/10] modetest: add the ability to specify fill patterns on the commandline
Instead of hacking the binary every time, we can now specify directly.
Signed-off-by: Ilia Mirkin
---
tests/modetest/modetest.c | 29 -
tests/util/pattern.c | 20
tests/util/pattern.h | 2 ++
3 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 7bb21d17..e66be660 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -67,6 +67,9 @@
#include "buffers.h"
#include "cursor.h"
+static enum util_fill_pattern primary_fill = UTIL_PATTERN_SMPTE;
+static enum util_fill_pattern secondary_fill = UTIL_PATTERN_TILES;
+
struct crtc {
drmModeCrtc *crtc;
drmModeObjectProperties *props;
@@ -1259,7 +1262,7 @@ static int set_plane(struct device *dev, struct plane_arg
*p)
p->w, p->h, p->format_str, plane_id);
plane_bo = bo_create(dev->fd, p->fourcc, p->w, p->h, handles,
-pitches, offsets, UTIL_PATTERN_TILES);
+pitches, offsets, secondary_fill);
if (plane_bo == NULL)
return -1;
@@ -1300,12 +1303,12 @@ static int set_plane(struct device *dev, struct
plane_arg *p)
static void atomic_set_planes(struct device *dev, struct plane_arg *p,
unsigned int count, bool update)
{
- unsigned int i, pattern = UTIL_PATTERN_SMPTE;
+ unsigned int i, pattern = primary_fill;
/* set up planes */
for (i = 0; i < count; i++) {
if (i > 0)
- pattern = UTIL_PATTERN_TILES;
+ pattern = secondary_fill;
else
set_gamma(dev, p[i].crtc_id, p[i].fourcc);
@@ -1450,7 +1453,7 @@ static void set_mode(struct device *dev, struct pipe_arg
*pipes, unsigned int co
bo = bo_create(dev->fd, pipes[0].fourcc, dev->mode.width,
dev->mode.height, handles, pitches, offsets,
- UTIL_PATTERN_SMPTE);
+ primary_fill);
if (bo == NULL)
return;
@@ -1794,6 +1797,18 @@ static int parse_property(struct property_arg *p, const
char *arg)
return 0;
}
+static void parse_fill_patterns(char *arg)
+{
+ char *fill = strtok(arg, ",");
+ if (!fill)
+ return;
+ primary_fill = util_pattern_enum(fill);
+ fill = strtok(NULL, ",");
+ if (!fill)
+ return;
+ secondary_fill = util_pattern_enum(fill);
+}
+
static void usage(char *name)
{
fprintf(stderr, "usage: %s [-acDdefMPpsCvw]\n", name);
@@ -1811,6 +1826,7 @@ static void usage(char *name)
fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
fprintf(stderr, "\t-w ::\tset property\n");
fprintf(stderr, "\t-a \tuse atomic API\n");
+ fprintf(stderr, "\t-F pattern1,pattern2\tspecify fill patterns\n");
fprintf(stderr, "\n Generic options:\n\n");
fprintf(stderr, "\t-d\tdrop master after mode set\n");
@@ -1874,7 +1890,7 @@ static int pipe_resolve_connectors(struct device *dev,
struct pipe_arg *pipe)
return 0;
}
-static char optstr[] = "acdD:efM:P:ps:Cvw:";
+static char optstr[] = "acdD:efF:M:P:ps:Cvw:";
int main(int argc, char **argv)
{
@@ -1923,6 +1939,9 @@ int main(int argc, char **argv)
case 'f':
framebuffers = 1;
break;
+ case 'F':
+ parse_fill_patterns(optarg);
+ break;
case 'M':
module = optarg;
/* Preserve the default behaviour of dumping all
information. */
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index d197c444..42a0e5c7 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -35,6 +35,7 @@
#include
#endif
+#include "common.h"
#include "format.h"
#include "pattern.h"
@@ -1261,3 +1262,22 @@ void util_fill_pattern(uint32_t format, enum
util_fill_pattern pattern,
break;
}
}
+
+static const char *pattern_names[] = {
+ [UTIL_PATTERN_TILES] = "tiles",
+ [UTIL_PATTERN_SMPTE] = "smpte",
+ [UTIL_PATTERN_PLAIN] = "plain",
+ [UTIL_PATTERN_GRADIENT] = "gradient",
+};
+
+enum util_fill_pattern util_pattern_enum(const char *name)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(pattern_names); i++)
+ if (!strcmp(pattern_names[i], name))
+ return (enum util_fill_pattern)i;
+
+ printf("Error: unsupported test pattern %s.\n", name);
+ return UTIL_PATTERN_SMPTE;
+}
diff --git a/tests
[PATCH libdrm 07/10] modetest: add an add_property_optional variant that does not print errors
As new features are added and others are declared to be legacy, it's
nice to be able to implement fallbacks. As such, create a
property-setting variant that does not generate errors which can very
well be entirely expected.
Will be used for gamma control in a future change.
Signed-off-by: Ilia Mirkin
---
tests/modetest/modetest.c | 29 +++--
1 file changed, 23 insertions(+), 6 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index a1c81f6c..71ddc861 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -948,9 +948,10 @@ struct property_arg {
char name[DRM_PROP_NAME_LEN+1];
uint32_t prop_id;
uint64_t value;
+ bool optional;
};
-static void set_property(struct device *dev, struct property_arg *p)
+static bool set_property(struct device *dev, struct property_arg *p)
{
drmModeObjectProperties *props = NULL;
drmModePropertyRes **props_info = NULL;
@@ -982,13 +983,13 @@ static void set_property(struct device *dev, struct
property_arg *p)
if (p->obj_type == 0) {
fprintf(stderr, "Object %i not found, can't set property\n",
p->obj_id);
- return;
+ return false;
}
if (!props) {
fprintf(stderr, "%s %i has no properties\n",
obj_type, p->obj_id);
- return;
+ return false;
}
for (i = 0; i < (int)props->count_props; ++i) {
@@ -999,9 +1000,10 @@ static void set_property(struct device *dev, struct
property_arg *p)
}
if (i == (int)props->count_props) {
- fprintf(stderr, "%s %i has no %s property\n",
- obj_type, p->obj_id, p->name);
- return;
+ if (!p->optional)
+ fprintf(stderr, "%s %i has no %s property\n",
+ obj_type, p->obj_id, p->name);
+ return false;
}
p->prop_id = props->props[i];
@@ -1015,6 +1017,8 @@ static void set_property(struct device *dev, struct
property_arg *p)
if (ret < 0)
fprintf(stderr, "failed to set %s %i property %s to %" PRIu64
": %s\n",
obj_type, p->obj_id, p->name, p->value,
strerror(errno));
+
+ return true;
}
/* --
*/
@@ -1072,6 +1076,19 @@ static void add_property(struct device *dev, uint32_t
obj_id,
set_property(dev, &p);
}
+static bool add_property_optional(struct device *dev, uint32_t obj_id,
+ const char *name, uint64_t value)
+{
+ struct property_arg p;
+
+ p.obj_id = obj_id;
+ strcpy(p.name, name);
+ p.value = value;
+ p.optional = true;
+
+ return set_property(dev, &p);
+}
+
static int atomic_set_plane(struct device *dev, struct plane_arg *p,
int pattern, bool
update)
{
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 06/10] modetest: don't pretend that atomic mode includes a format
Signed-off-by: Ilia Mirkin
---
tests/modetest/modetest.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
index 9c85c07b..a1c81f6c 100644
--- a/tests/modetest/modetest.c
+++ b/tests/modetest/modetest.c
@@ -1335,8 +1335,8 @@ static void atomic_set_mode(struct device *dev, struct
pipe_arg *pipes, unsigned
if (pipe->mode == NULL)
continue;
- printf("setting mode %s-%dHz@%s on connectors ",
- pipe->mode_str, pipe->mode->vrefresh, pipe->format_str);
+ printf("setting mode %s-%dHz on connectors ",
+ pipe->mode_str, pipe->mode->vrefresh);
for (j = 0; j < pipe->num_cons; ++j) {
printf("%s, ", pipe->cons[j]);
add_property(dev, pipe->con_ids[j], "CRTC_ID",
pipe->crtc->crtc->crtc_id);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 04/10] util: add fp16 format support
This change adds support for all current patterns.
Signed-off-by: Ilia Mirkin
---
tests/util/format.c | 5 ++
tests/util/pattern.c | 207 ++-
2 files changed, 209 insertions(+), 3 deletions(-)
diff --git a/tests/util/format.c b/tests/util/format.c
index e52213bb..1ca1b82c 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -93,6 +93,11 @@ static const struct util_format_info format_info[] = {
{ DRM_FORMAT_RGBX1010102, "RX30", MAKE_RGB_INFO(10, 22, 10, 12, 10, 2,
0, 0) },
{ DRM_FORMAT_BGRA1010102, "BA30", MAKE_RGB_INFO(10, 2, 10, 12, 10, 22,
2, 0) },
{ DRM_FORMAT_BGRX1010102, "BX30", MAKE_RGB_INFO(10, 2, 10, 12, 10, 22,
0, 0) },
+ { DRM_FORMAT_XRGB16161616F, "XR4H", MAKE_RGB_INFO(16, 32, 16, 16, 16,
0, 0, 0) },
+ { DRM_FORMAT_XBGR16161616F, "XB4H", MAKE_RGB_INFO(16, 0, 16, 16, 16,
32, 0, 0) },
+ { DRM_FORMAT_ARGB16161616F, "AR4H", MAKE_RGB_INFO(16, 32, 16, 16, 16,
0, 16, 48) },
+ { DRM_FORMAT_ABGR16161616F, "AB4H", MAKE_RGB_INFO(16, 0, 16, 16, 16,
32, 16, 48) },
+
};
uint32_t util_format_fourcc(const char *name)
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index ff110fd7..37796dbf 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -86,6 +86,17 @@ static inline uint32_t shiftcolor10(const struct
util_color_component *comp,
return value << comp->offset;
}
+/* This function takes 16-bit color values */
+static inline uint64_t shiftcolor16(const struct util_color_component *comp,
+ uint64_t value)
+{
+ value &= 0x;
+ /* Shift down to remove unwanted low bits */
+ value = value >> (16 - comp->length);
+ /* Shift back up to where the value should be */
+ return value << comp->offset;
+}
+
#define MAKE_RGBA10(rgb, r, g, b, a) \
(shiftcolor10(&(rgb)->red, (r)) | \
shiftcolor10(&(rgb)->green, (g)) | \
@@ -101,6 +112,49 @@ static inline uint32_t shiftcolor10(const struct
util_color_component *comp,
#define MAKE_RGB24(rgb, r, g, b) \
{ .value = MAKE_RGBA(rgb, r, g, b, 0) }
+
+/**
+ * Takes a uint16_t, divides by 65536, converts the infinite-precision
+ * result to fp16 with round-to-zero.
+ *
+ * Copied from mesa:src/util/half_float.c
+ */
+static uint16_t uint16_div_64k_to_half(uint16_t v)
+{
+ /* Zero or subnormal. Set the mantissa to (v << 8) and return. */
+ if (v < 4)
+ return v << 8;
+
+ /* Count the leading 0s in the uint16_t */
+ int n = __builtin_clz(v) - 16;
+
+ /* Shift the mantissa up so bit 16 is the hidden 1 bit,
+* mask it off, then shift back down to 10 bits
+*/
+ int m = ( ((uint32_t)v << (n + 1)) & 0x ) >> 6;
+
+ /* (0{n} 1 X{15-n}) * 2^-16
+* = 1.X * 2^(15-n-16)
+* = 1.X * 2^(14-n - 15)
+* which is the FP16 form with e = 14 - n
+*/
+ int e = 14 - n;
+
+ return (e << 10) | m;
+}
+
+#define MAKE_RGBA8FP16(rgb, r, g, b, a) \
+ (shiftcolor16(&(rgb)->red, uint16_div_64k_to_half((r) << 8)) | \
+shiftcolor16(&(rgb)->green, uint16_div_64k_to_half((g) << 8)) | \
+shiftcolor16(&(rgb)->blue, uint16_div_64k_to_half((b) << 8)) | \
+shiftcolor16(&(rgb)->alpha, uint16_div_64k_to_half((a) << 8)))
+
+#define MAKE_RGBA10FP16(rgb, r, g, b, a) \
+ (shiftcolor16(&(rgb)->red, uint16_div_64k_to_half((r) << 6)) | \
+shiftcolor16(&(rgb)->green, uint16_div_64k_to_half((g) << 6)) | \
+shiftcolor16(&(rgb)->blue, uint16_div_64k_to_half((b) << 6)) | \
+shiftcolor16(&(rgb)->alpha, uint16_div_64k_to_half((a) << 6)))
+
static void fill_smpte_yuv_planar(const struct util_yuv_info *yuv,
unsigned char *y_mem, unsigned char *u_mem,
unsigned char *v_mem, unsigned int width,
@@ -489,6 +543,67 @@ static void fill_smpte_rgb32(const struct util_rgb_info
*rgb, void *mem,
}
}
+static void fill_smpte_rgb16fp(const struct util_rgb_info *rgb, void *mem,
+ unsigned int width, unsigned int height,
+ unsigned int stride)
+{
+ const uint64_t colors_top[] = {
+ MAKE_RGBA8FP16(rgb, 192, 192, 192, 255),/* grey */
+ MAKE_RGBA8FP16(rgb, 192, 192, 0, 255), /* yellow */
+ MAKE_RGBA8FP16(rgb, 0, 192, 192, 255), /* cyan */
+ MAKE_RGBA8FP16(rgb, 0, 192, 0, 255),/* green */
+ MAKE_RGBA8FP16(rgb, 192, 0, 192, 255), /* magenta */
+ MAKE_RGBA8FP16(rgb, 192, 0, 0, 255),/* red */
+ MAKE_RGBA8FP16(rgb, 0, 0, 192, 255),/* blue */
[PATCH libdrm 05/10] util: add cairo drawing for 30bpp formats when available
Signed-off-by: Ilia Mirkin --- tests/util/pattern.c | 8 1 file changed, 8 insertions(+) diff --git a/tests/util/pattern.c b/tests/util/pattern.c index 37796dbf..d197c444 100644 --- a/tests/util/pattern.c +++ b/tests/util/pattern.c @@ -788,6 +788,14 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height, case DRM_FORMAT_BGR565: cairo_format = CAIRO_FORMAT_RGB16_565; break; +#if CAIRO_VERSION_MAJOR > 1 || (CAIRO_VERSION_MAJOR == 1 && CAIRO_VERSION_MINOR >= 12) + case DRM_FORMAT_ARGB2101010: + case DRM_FORMAT_XRGB2101010: + case DRM_FORMAT_ABGR2101010: + case DRM_FORMAT_XBGR2101010: + cairo_format = CAIRO_FORMAT_RGB30; + break; +#endif default: return; } -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 10/10] modetest: add FP16 format support
Signed-off-by: Ilia Mirkin --- tests/modetest/buffers.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/tests/modetest/buffers.c b/tests/modetest/buffers.c index 5ec4ec8e..8a8d9e01 100644 --- a/tests/modetest/buffers.c +++ b/tests/modetest/buffers.c @@ -194,6 +194,13 @@ bo_create(int fd, unsigned int format, bpp = 32; break; + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_XBGR16161616F: + case DRM_FORMAT_ARGB16161616F: + case DRM_FORMAT_ABGR16161616F: + bpp = 64; + break; + default: fprintf(stderr, "unsupported format 0x%08x\n", format); return NULL; @@ -313,6 +320,10 @@ bo_create(int fd, unsigned int format, case DRM_FORMAT_RGBX1010102: case DRM_FORMAT_BGRA1010102: case DRM_FORMAT_BGRX1010102: + case DRM_FORMAT_XRGB16161616F: + case DRM_FORMAT_XBGR16161616F: + case DRM_FORMAT_ARGB16161616F: + case DRM_FORMAT_ABGR16161616F: offsets[0] = 0; handles[0] = bo->handle; pitches[0] = bo->pitch; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 00/10] Add C8, 30bpp and FP16 support to modetest
On Mon, Jun 3, 2019 at 3:32 AM Daniel Vetter wrote: > > On Sun, Jun 02, 2019 at 08:40:08PM -0400, Ilia Mirkin wrote: > > This series improves the pattern generation logic to support additional > > formats, as well as a new "gradient" pattern (see patch comments on why > > I found it useful). > > > > Furthermore, these formats are piped through to modetest, including the > > ability to set a gamma table, which is necessary for the C8 indexed > > format. > > > > This was tested on nouveau, and used for bring-up of the C8, XB30, and > > FP16 formats on the NVIDIA hardware that supports these. > > Does nouveau also work with igt tests for this stuff? We do have support > for interactive testing (i.e. "human pls check yourself" kind of tests) in > igt, so ideally we could merge everything into one place. Long-term at > least ... nouveau has no special exclusions for programs that start with the letters "igt", so presumably it should be OK with the basic tests. However it was my impression that igt was targeted at automated testing, and all the tests basically required crc, which is questionable whether it exists in the hw in a manner usable by such tests, and definitely not supported by nouveau in any case. As a result, I haven't really taken much of a look. Having something flexible like modetest has been really useful in development. Being able to run with different formats, messing with resolutions, scaling parameters for overlays, different patterns -- these things have all been helpful in validating that the new features implemented actually work as expected. I plan on extending it further to cover HDR, as part of my bringup of HDR on nouveau. As an example, pre-GF119 FP16 support expects a 0..1024-valued input instead of 0..1 (something which we did not previously know). I was able to guess that by changing the pattern in the code to generate larger numbers, after seeing a black display with the 0..1 pattern. (I may have also messed with the gamma ramp to see if it was "working" or not - I forget already.) Having a tool that makes things like that simple to investigate is pretty valuable to me. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 00/10] Add C8, 30bpp and FP16 support to modetest
On Sun, Jun 2, 2019 at 8:40 PM Ilia Mirkin wrote: > > This series improves the pattern generation logic to support additional > formats, as well as a new "gradient" pattern (see patch comments on why > I found it useful). > > Furthermore, these formats are piped through to modetest, including the > ability to set a gamma table, which is necessary for the C8 indexed > format. > > This was tested on nouveau, and used for bring-up of the C8, XB30, and > FP16 formats on the NVIDIA hardware that supports these. Just to follow up, I've successfully tested on an Intel SKL with C8 and XB30/XR30 as well (and confirmed that the GAMMA_LUT gets unset in a sequence of C8 followed by XB30). FP16 was not available on the kernel I am currently using (and perhaps not the HW?) -ilia > > Ilia Mirkin (10): > util: add C8 format, support it with SMPTE pattern > util: fix MAKE_RGBA macro for 10bpp modes > util: add gradient pattern > util: add fp16 format support > util: add cairo drawing for 30bpp formats when available > modetest: don't pretend that atomic mode includes a format > modetest: add an add_property_optional variant that does not print > errors > modetest: add C8 support to generate SMPTE pattern > modetest: add the ability to specify fill patterns on the commandline > modetest: add FP16 format support > > tests/modetest/buffers.c | 13 ++ > tests/modetest/modetest.c | 109 -- > tests/util/format.c | 7 + > tests/util/pattern.c | 432 +- > tests/util/pattern.h | 7 + > 5 files changed, 543 insertions(+), 25 deletions(-) > > -- > 2.21.0 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 06/10] modetest: don't pretend that atomic mode includes a format
On Thu, Jun 6, 2019 at 11:51 AM Emil Velikov wrote:
>
> On Mon, 3 Jun 2019 at 01:40, Ilia Mirkin wrote:
> >
> > Signed-off-by: Ilia Mirkin
> > ---
> > tests/modetest/modetest.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > index 9c85c07b..a1c81f6c 100644
> > --- a/tests/modetest/modetest.c
> > +++ b/tests/modetest/modetest.c
> > @@ -1335,8 +1335,8 @@ static void atomic_set_mode(struct device *dev,
> > struct pipe_arg *pipes, unsigned
> > if (pipe->mode == NULL)
> > continue;
> >
> > - printf("setting mode %s-%dHz@%s on connectors ",
> > - pipe->mode_str, pipe->mode->vrefresh,
> > pipe->format_str);
> > + printf("setting mode %s-%dHz on connectors ",
> > + pipe->mode_str, pipe->mode->vrefresh);
>
> AFAICT we can drop the format on modeset all together. I cannot see
> anything that would require it - regardless if the modeset is atomic
> or not.
> Plus we can remove the --help string and argument parsing code.
>
> Can I interest you in doing that?
The format plays with a non-atomic modeset. It's used for the fb that
gets added.
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/nouveau/kms/gf119-: add ctm property support
This adds support on GF119:GV100 (exclusive) for CTM (aka CSC).
Signed-off-by: Ilia Mirkin
---
drivers/gpu/drm/nouveau/dispnv50/atom.h | 6 ++
drivers/gpu/drm/nouveau/dispnv50/base907c.c | 65 +
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 13 +
drivers/gpu/drm/nouveau/dispnv50/wndw.h | 4 ++
4 files changed, 88 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h
b/drivers/gpu/drm/nouveau/dispnv50/atom.h
index b5fae5ab3fa8..894d1fec6f0a 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/atom.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h
@@ -184,6 +184,11 @@ struct nv50_wndw_atom {
} i;
} xlut;
+ struct {
+ u32 matrix[12];
+ bool valid;
+ } ctm;
+
struct {
u8 mode:2;
u8 interval:4;
@@ -221,6 +226,7 @@ struct nv50_wndw_atom {
bool ntfy:1;
bool sema:1;
bool xlut:1;
+ bool ctm:1;
bool image:1;
bool scale:1;
bool point:1;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
index 049ce6da321c..ceadc2e3efe9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
@@ -83,6 +83,68 @@ base907c_ilut(struct nv50_wndw *wndw, struct nv50_wndw_atom
*asyw)
asyw->xlut.i.load = head907d_olut_load;
}
+static inline u32
+ctm_drm_to_base(u64 in)
+{
+ /* base takes a 19-bit 2's complement value in S3.16 format */
+ bool sign = in & BIT_ULL(63);
+ u32 integer = (in >> 32) & 0x7fff;
+ u32 fraction = in & 0x;
+
+ if (integer >= 4) {
+ return (1 << 18) - (sign ? 0 : 1);
+ } else {
+ u32 ret = (integer << 16) | (fraction >> 16);
+ if (sign)
+ ret = -ret;
+ return ret & GENMASK(18, 0);
+ }
+}
+
+static void
+base907c_ctm(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
+const struct drm_color_ctm *ctm)
+{
+ int i, j;
+
+ for (j = 0; j < 3; j++) {
+ for (i = 0; i < 4; i++) {
+ u32 *val = &asyw->ctm.matrix[j * 4 + i];
+ /* CTM does not support constant offset, while
+* HW CSC does. Skip it. */
+ if (i == 3) {
+ *val = 0;
+ } else {
+ *val = ctm_drm_to_base(ctm->matrix[j * 3 + i]);
+ }
+ }
+ }
+}
+
+static void
+base907c_ctm_clr(struct nv50_wndw *wndw)
+{
+ u32 *push;
+ if ((push = evo_wait(&wndw->wndw, 2))) {
+ evo_mthd(push, 0x0140, 1);
+ evo_data(push, 0x);
+ evo_kick(push, &wndw->wndw);
+ }
+}
+
+static void
+base907c_ctm_set(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw)
+{
+ u32 *push, i;
+ if ((push = evo_wait(&wndw->wndw, 13))) {
+ evo_mthd(push, 0x0140, 12);
+ evo_data(push, asyw->ctm.matrix[0] | 0x8000);
+ for (i = 1; i < 12; i++)
+ evo_data(push, asyw->ctm.matrix[i]);
+ evo_kick(push, &wndw->wndw);
+ }
+}
+
const struct nv50_wndw_func
base907c = {
.acquire = base507c_acquire,
@@ -94,6 +156,9 @@ base907c = {
.ntfy_clr = base507c_ntfy_clr,
.ntfy_wait_begun = base507c_ntfy_wait_begun,
.ilut = base907c_ilut,
+ .ctm = base907c_ctm,
+ .ctm_set = base907c_ctm_set,
+ .ctm_clr = base907c_ctm_clr,
.olut_core = true,
.xlut_set = base907c_xlut_set,
.xlut_clr = base907c_xlut_clr,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 3bee1f063dfb..2ae3ca9a5968 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -118,6 +118,7 @@ nv50_wndw_flush_clr(struct nv50_wndw *wndw, u32 *interlock,
bool flush,
if (clr.sema ) wndw->func-> sema_clr(wndw);
if (clr.ntfy ) wndw->func-> ntfy_clr(wndw);
if (clr.xlut ) wndw->func-> xlut_clr(wndw);
+ if (clr.ctm ) wndw->func-> ctm_clr(wndw);
if (clr.image) wndw->func->image_clr(wndw);
interlock[wndw->interlock.type] |= wndw->interlock.data;
@@ -145,6 +146,7 @@ nv50_wndw_flush_set(struct nv50_wndw *wndw, u32 *interlock,
wndw->func->xlut_set(wndw, asyw);
}
+ if (asyw->set.ctm ) wndw->func->ctm_set (wndw, asyw);
if (asyw->set.scale) wndw->func->scale_set(wndw, asyw);
if (asyw->set.point) {
[PATCH 2/2] drm/nouveau/kms/nv50-: enable modern color management properties
For GF119:GV100, we can enable DEGAMMA/CTM/GAMMA. For earlier GPUs, as
there is no CTM, having both degamma and gamma is a bit pointless. Later
GPUs currently lack an implementation.
Signed-off-by: Ilia Mirkin
---
drivers/gpu/drm/nouveau/dispnv50/head.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 06ee23823a68..f42aa79f2f40 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -499,6 +499,11 @@ nv50_head_create(struct drm_device *dev, int index)
&nv50_head_func, "head-%d", head->base.index);
drm_crtc_helper_add(crtc, &nv50_head_help);
drm_mode_crtc_set_gamma_size(crtc, 256);
+ if (disp->disp->object.oclass >= GF110_DISP &&
+ disp->disp->object.oclass < GV100_DISP)
+ drm_crtc_enable_color_mgmt(crtc, 256, true, 256);
+ else
+ drm_crtc_enable_color_mgmt(crtc, 0, false, 256);
if (head->func->olut_set) {
ret = nv50_lut_init(disp, &drm->client.mmu, &head->olut);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/nouveau/kms/gf119-: add ctm property support
This adds support on GF119:GV100 (exclusive) for CTM (aka CSC).
Signed-off-by: Ilia Mirkin
---
v1 -> v2:
- ctm -> csc
- mark csc.valid = false when there is no ctm property
drivers/gpu/drm/nouveau/dispnv50/atom.h | 6 ++
drivers/gpu/drm/nouveau/dispnv50/base907c.c | 65 +
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 16 +
drivers/gpu/drm/nouveau/dispnv50/wndw.h | 4 ++
4 files changed, 91 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h
b/drivers/gpu/drm/nouveau/dispnv50/atom.h
index b5fae5ab3fa8..75bda111da10 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/atom.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h
@@ -184,6 +184,11 @@ struct nv50_wndw_atom {
} i;
} xlut;
+ struct {
+ u32 matrix[12];
+ bool valid;
+ } csc;
+
struct {
u8 mode:2;
u8 interval:4;
@@ -221,6 +226,7 @@ struct nv50_wndw_atom {
bool ntfy:1;
bool sema:1;
bool xlut:1;
+ bool csc:1;
bool image:1;
bool scale:1;
bool point:1;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
index 049ce6da321c..fd0c1d84730b 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
@@ -83,6 +83,68 @@ base907c_ilut(struct nv50_wndw *wndw, struct nv50_wndw_atom
*asyw)
asyw->xlut.i.load = head907d_olut_load;
}
+static inline u32
+csc_drm_to_base(u64 in)
+{
+ /* base takes a 19-bit 2's complement value in S3.16 format */
+ bool sign = in & BIT_ULL(63);
+ u32 integer = (in >> 32) & 0x7fff;
+ u32 fraction = in & 0x;
+
+ if (integer >= 4) {
+ return (1 << 18) - (sign ? 0 : 1);
+ } else {
+ u32 ret = (integer << 16) | (fraction >> 16);
+ if (sign)
+ ret = -ret;
+ return ret & GENMASK(18, 0);
+ }
+}
+
+static void
+base907c_csc(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw,
+const struct drm_color_ctm *ctm)
+{
+ int i, j;
+
+ for (j = 0; j < 3; j++) {
+ for (i = 0; i < 4; i++) {
+ u32 *val = &asyw->csc.matrix[j * 4 + i];
+ /* DRM does not support constant offset, while
+* HW CSC does. Skip it. */
+ if (i == 3) {
+ *val = 0;
+ } else {
+ *val = csc_drm_to_base(ctm->matrix[j * 3 + i]);
+ }
+ }
+ }
+}
+
+static void
+base907c_csc_clr(struct nv50_wndw *wndw)
+{
+ u32 *push;
+ if ((push = evo_wait(&wndw->wndw, 2))) {
+ evo_mthd(push, 0x0140, 1);
+ evo_data(push, 0x);
+ evo_kick(push, &wndw->wndw);
+ }
+}
+
+static void
+base907c_csc_set(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw)
+{
+ u32 *push, i;
+ if ((push = evo_wait(&wndw->wndw, 13))) {
+ evo_mthd(push, 0x0140, 12);
+ evo_data(push, asyw->csc.matrix[0] | 0x8000);
+ for (i = 1; i < 12; i++)
+ evo_data(push, asyw->csc.matrix[i]);
+ evo_kick(push, &wndw->wndw);
+ }
+}
+
const struct nv50_wndw_func
base907c = {
.acquire = base507c_acquire,
@@ -94,6 +156,9 @@ base907c = {
.ntfy_clr = base507c_ntfy_clr,
.ntfy_wait_begun = base507c_ntfy_wait_begun,
.ilut = base907c_ilut,
+ .csc = base907c_csc,
+ .csc_set = base907c_csc_set,
+ .csc_clr = base907c_csc_clr,
.olut_core = true,
.xlut_set = base907c_xlut_set,
.xlut_clr = base907c_xlut_clr,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
index 3bee1f063dfb..76843e8231c1 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
@@ -118,6 +118,7 @@ nv50_wndw_flush_clr(struct nv50_wndw *wndw, u32 *interlock,
bool flush,
if (clr.sema ) wndw->func-> sema_clr(wndw);
if (clr.ntfy ) wndw->func-> ntfy_clr(wndw);
if (clr.xlut ) wndw->func-> xlut_clr(wndw);
+ if (clr.csc ) wndw->func-> csc_clr(wndw);
if (clr.image) wndw->func->image_clr(wndw);
interlock[wndw->interlock.type] |= wndw->interlock.data;
@@ -145,6 +146,7 @@ nv50_wndw_flush_set(struct nv50_wndw *wndw, u32 *interlock,
wndw->func->xlut_set(wndw, asyw);
}
+ if (asyw->set.csc ) wndw->func->csc_set (wndw, asyw);
if
Re: [RFC/WIP] drm/rockchip: Support CRTC gamma LUT
Note that userspace may provide any size of gamma lut. Have a look at
i915/intel_color.c:intel_color_check which filters out only the
allowed sizes. Consider having a special allowance for 256-sized LUTs
since that's what most legacy userspace will set, and it seems like a
waste to create a 10-bit LUT for RGBA8 color.
-ilia
On Thu, Jun 13, 2019 at 3:23 PM Ezequiel Garcia wrote:
>
> Add CRTC gamma LUT configuration on RK3288 and RK3399.
>
> Signed-off-by: Ezequiel Garcia
> ---
> This patch seems to work well on RK3288, but produces
> a distorted output on RK3399. I was hoping
> someone could have any idea, so we can support both
> platforms.
>
> drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 87 +
> drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 2 +
> drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 4 +
> drivers/gpu/drm/rockchip/rockchip_vop_reg.h | 1 +
> 4 files changed, 94 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 12ed5265a90b..8381679c1045 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -38,6 +38,8 @@
> #include "rockchip_drm_vop.h"
> #include "rockchip_rgb.h"
>
> +#define VOP_GAMMA_LUT_SIZE 1024
> +
> #define VOP_WIN_SET(vop, win, name, v) \
> vop_reg_set(vop, &win->phy->name, win->base, ~0, v, #name)
> #define VOP_SCL_SET(vop, win, name, v) \
> @@ -137,6 +139,7 @@ struct vop {
>
> uint32_t *regsbak;
> void __iomem *regs;
> + void __iomem *lut_regs;
>
> /* physical map length of vop register */
> uint32_t len;
> @@ -1153,6 +1156,46 @@ static void vop_wait_for_irq_handler(struct vop *vop)
> synchronize_irq(vop->irq);
> }
>
> +static bool vop_dsp_lut_is_enable(struct vop *vop)
> +{
> + return vop_read_reg(vop, 0, &vop->data->common->dsp_lut_en);
> +}
> +
> +static void vop_crtc_gamma_set(struct vop *vop, struct drm_crtc *crtc,
> + struct drm_crtc_state *state)
> +{
> + struct drm_color_lut *lut;
> + int i, idle, ret;
> +
> + if (!state->gamma_lut)
> + return;
> + lut = state->gamma_lut->data;
> +
> + spin_lock(&vop->reg_lock);
> + VOP_REG_SET(vop, common, dsp_lut_en, 0);
> + vop_cfg_done(vop);
> + spin_unlock(&vop->reg_lock);
> +
> + ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop,
> + idle, !idle, 5, 10 * 3);
> + if (ret)
> + return;
> +
> + spin_lock(&vop->reg_lock);
> + for (i = 0; i < crtc->gamma_size; i++) {
> + u32 word;
> +
> + word = (drm_color_lut_extract(lut[i].red, 10) << 20) |
> + (drm_color_lut_extract(lut[i].green, 10) << 10) |
> + drm_color_lut_extract(lut[i].blue, 10);
> + writel(word, vop->lut_regs + i * 4);
> + }
> +
> + VOP_REG_SET(vop, common, dsp_lut_en, 1);
> + vop_cfg_done(vop);
> + spin_unlock(&vop->reg_lock);
> +}
> +
> static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> struct drm_crtc_state *old_crtc_state)
> {
> @@ -1201,6 +1244,9 @@ static void vop_crtc_atomic_flush(struct drm_crtc *crtc,
> drm_flip_work_queue(&vop->fb_unref_work, old_plane_state->fb);
> set_bit(VOP_PENDING_FB_UNREF, &vop->pending);
> }
> +
> + if (vop->lut_regs && crtc->state->color_mgmt_changed)
> + vop_crtc_gamma_set(vop, crtc, crtc->state);
> }
>
> static const struct drm_crtc_helper_funcs vop_crtc_helper_funcs = {
> @@ -1323,6 +1369,7 @@ static const struct drm_crtc_funcs vop_crtc_funcs = {
> .disable_vblank = vop_crtc_disable_vblank,
> .set_crc_source = vop_crtc_set_crc_source,
> .verify_crc_source = vop_crtc_verify_crc_source,
> + .gamma_set = drm_atomic_helper_legacy_gamma_set,
> };
>
> static void vop_fb_unref_worker(struct drm_flip_work *work, void *val)
> @@ -1480,6 +1527,8 @@ static int vop_create_crtc(struct vop *vop)
> goto err_cleanup_planes;
>
> drm_crtc_helper_add(crtc, &vop_crtc_helper_funcs);
> + drm_mode_crtc_set_gamma_size(crtc, VOP_GAMMA_LUT_SIZE);
> + drm_crtc_enable_color_mgmt(crtc, 0, false, VOP_GAMMA_LUT_SIZE);
>
> /*
> * Create drm_planes for overlay windows with possible_crtcs
> restricted
> @@ -1744,6 +1793,41 @@ int rockchip_drm_wait_vact_end(struct drm_crtc *crtc,
> unsigned int mstimeout)
> }
> EXPORT_SYMBOL(rockchip_drm_wait_vact_end);
>
> +static int vop_gamma_lut_request(struct device *dev,
> +struct resource *res, struct vop *vop)
> +{
> + resource_size_t offset = vop->data->gamma_lut_addr_off;
> + resource_size_t size = VOP_GAMMA_LUT_SIZE * 4;
> +
> + /*
> +* Some SoCs (e.g. RK3288) have the gamma LUT addr
Re: [RFC/WIP] drm/rockchip: Support CRTC gamma LUT
On Tue, Jun 18, 2019 at 9:36 AM Ezequiel Garcia wrote: > > On Thu, 2019-06-13 at 15:36 -0400, Ilia Mirkin wrote: > > Note that userspace may provide any size of gamma lut. Have a look at > > i915/intel_color.c:intel_color_check which filters out only the > > allowed sizes. Consider having a special allowance for 256-sized LUTs > > since that's what most legacy userspace will set, and it seems like a > > waste to create a 10-bit LUT for RGBA8 color. > > > > Right. I will add a check for the gamma lut size. > > Unfortunately, this hardware seems to only support 10-bit, 1024-sized LUTs. > > The spec does mention a support 8-bit, 256-entries, but it's not at all > clear how configure that. It's up to you, and the more experienced drm reviewers, but even if you can't figure out how to bend the hardware to your will (which is worth a bit of exploration), you can still emulate it by just repeating all the values 4x. IMHO 256-sized LUTs are worth supporting when possible. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/rockchip: Add optional support for CRTC gamma LUT
On Tue, Jun 18, 2019 at 5:43 PM Ezequiel Garcia wrote:
>
> Add an optional CRTC gamma LUT support, and enable it on RK3288.
> This is currently enabled via a separate address resource,
> which needs to be specified in the devicetree.
>
> The address resource is required because on some SoCs, such as
> RK3288, the LUT address is after the MMU address, and the latter
> is supported by a different driver. This prevents the DRM driver
> from requesting an entire register space.
>
> The current implementation works for RGB 10-bit tables, as that
> is what seems to work on RK3288.
>
> Signed-off-by: Ezequiel Garcia
> ---
> Changes from RFC:
> * Request (an optional) address resource for the LUT.
> * Drop support for RK3399, which doesn't seem to work
> out of the box and needs more research.
> * Support pass-thru setting when GAMMA_LUT is NULL.
> * Add a check for the gamma size, as suggested by Ilia.
> * Move gamma setting to atomic_commit_tail, as pointed
> out by Jacopo/Laurent, is the correct way.
> ---
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 12ed5265a90b..5b6edbe2673f 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +static void vop_crtc_gamma_set(struct vop *vop, struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> +{
> + int idle, ret, i;
> +
> + spin_lock(&vop->reg_lock);
> + VOP_REG_SET(vop, common, dsp_lut_en, 0);
> + vop_cfg_done(vop);
> + spin_unlock(&vop->reg_lock);
> +
> + ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop,
> + idle, !idle, 5, 30 * 1000);
> + if (ret)
> + return;
> +
> + spin_lock(&vop->reg_lock);
> +
> + if (crtc->state->gamma_lut) {
> + if (!old_state->gamma_lut || (crtc->state->gamma_lut->base.id
> !=
> + old_state->gamma_lut->base.id))
> + vop_crtc_write_gamma_lut(vop, crtc);
> + } else {
> + for (i = 0; i < crtc->gamma_size; i++) {
> + u32 word;
> +
> + word = (i << 20) | (i << 10) | i;
> + writel(word, vop->lut_regs + i * 4);
> + }
Note - I'm not in any way familiar with the hardware, so take with a
grain of salt --
Could you just leave dsp_lut_en turned off in this case?
Cheers,
-ilia
> + }
> +
> + VOP_REG_SET(vop, common, dsp_lut_en, 1);
> + vop_cfg_done(vop);
> + spin_unlock(&vop->reg_lock);
> +}
Re: nouveau: DRM: GPU lockup - switching to software fbcon
On Wed, Jun 19, 2019 at 1:08 AM Sergey Senozhatsky wrote: > > On (06/14/19 11:50), Sergey Senozhatsky wrote: > > dmesg > > > > nouveau :01:00.0: DRM: GPU lockup - switching to software fbcon > > nouveau :01:00.0: fifo: SCHED_ERROR 0a [CTXSW_TIMEOUT] > > nouveau :01:00.0: fifo: runlist 0: scheduled for recovery > > nouveau :01:00.0: fifo: channel 5: killed > > nouveau :01:00.0: fifo: engine 6: scheduled for recovery > > nouveau :01:00.0: fifo: engine 0: scheduled for recovery > > nouveau :01:00.0: firefox[476]: channel 5 killed! > > nouveau :01:00.0: firefox[476]: failed to idle channel 5 [firefox[476]] > > > > It lockups several times a day. Twice in just one hour today. > > Can we fix this? > > Unusable Are you using a GTX 660 by any chance? You've provided rather minimal system info. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: nouveau: DRM: GPU lockup - switching to software fbcon
On Wed, Jun 19, 2019 at 1:48 AM Sergey Senozhatsky wrote: > > On (06/19/19 01:20), Ilia Mirkin wrote: > > On Wed, Jun 19, 2019 at 1:08 AM Sergey Senozhatsky > > wrote: > > > > > > On (06/14/19 11:50), Sergey Senozhatsky wrote: > > > > dmesg > > > > > > > > nouveau :01:00.0: DRM: GPU lockup - switching to software fbcon > > > > nouveau :01:00.0: fifo: SCHED_ERROR 0a [CTXSW_TIMEOUT] > > > > nouveau :01:00.0: fifo: runlist 0: scheduled for recovery > > > > nouveau :01:00.0: fifo: channel 5: killed > > > > nouveau :01:00.0: fifo: engine 6: scheduled for recovery > > > > nouveau :01:00.0: fifo: engine 0: scheduled for recovery > > > > nouveau :01:00.0: firefox[476]: channel 5 killed! > > > > nouveau :01:00.0: firefox[476]: failed to idle channel 5 > > > > [firefox[476]] > > > > > > > > It lockups several times a day. Twice in just one hour today. > > > > Can we fix this? > > > > > > Unusable > > > > Are you using a GTX 660 by any chance? You've provided rather minimal > > system info. > > 01:00.0 VGA compatible controller: NVIDIA Corporation GK208B [GeForce GT 730] > (rev a1) Quite literally the same GPU I have plugged in... 02:00.0 VGA compatible controller [0300]: NVIDIA Corporation GK208B [GeForce GT 730] [10de:1287] (rev a1) Works great here! Only other thing I can think of is that I avoid applications with the letters "G" and "K" in their names, and I'm using xf86-video-nouveau ddx, whereas you might be using the "modeset" ddx with glamor. If all else fails, just remove nouveau_dri.so and/or boot with nouveau.noaccel=1 -- should be perfect. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/nouveau/kms/gf119-: allow both 256- and 1024-sized LUTs to be used
The hardware supports either size. Also add checks to ensure that only
these two sizes may be used for supplying a LUT.
Finally, experimental evidence suggests you can't mix sizes for degamma
and gamma. Disallow this as well.
Signed-off-by: Ilia Mirkin
---
Note that all the gv100+ changes are purely speculative.
Tested on a GK208 with 8- and 10-bpc formats (but still 8bpc output).
This is on top of the patches which I've sent in the past. You'll end up
with some conflicts, I suspect, but I can't rebase easily on your changes
since they're not in a linux tree.
drivers/gpu/drm/nouveau/dispnv50/base907c.c | 4 ++--
drivers/gpu/drm/nouveau/dispnv50/corec57d.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/head.c | 26 +
drivers/gpu/drm/nouveau/dispnv50/head.h | 10 +---
drivers/gpu/drm/nouveau/dispnv50/head507d.c | 10 +++-
drivers/gpu/drm/nouveau/dispnv50/head827d.c | 2 ++
drivers/gpu/drm/nouveau/dispnv50/head907d.c | 12 --
drivers/gpu/drm/nouveau/dispnv50/head917d.c | 2 ++
drivers/gpu/drm/nouveau/dispnv50/headc37d.c | 6 +++--
drivers/gpu/drm/nouveau/dispnv50/headc57d.c | 9 +++
drivers/gpu/drm/nouveau/dispnv50/lut.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.h | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 4 ++--
drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c | 3 +--
15 files changed, 70 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
index fd0c1d84730b..76db448205d2 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
@@ -76,9 +76,9 @@ base907c_xlut_set(struct nv50_wndw *wndw, struct
nv50_wndw_atom *asyw)
}
static void
-base907c_ilut(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw)
+base907c_ilut(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw, int size)
{
- asyw->xlut.i.mode = 7;
+ asyw->xlut.i.mode = size == 1024 ? 4 : 7;
asyw->xlut.i.enable = 2;
asyw->xlut.i.load = head907d_olut_load;
}
diff --git a/drivers/gpu/drm/nouveau/dispnv50/corec57d.c
b/drivers/gpu/drm/nouveau/dispnv50/corec57d.c
index b606d68cda10..700df4762b6d 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/corec57d.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/corec57d.c
@@ -36,7 +36,7 @@ corec57d_init(struct nv50_core *core)
evo_data(push, 0x000f);
evo_data(push, 0x);
evo_mthd(push, 0x1010 + (i * 0x080), 1);
- evo_data(push, 0x00117fff);
+ evo_data(push, 0x00127fff);
}
evo_mthd(push, 0x0200, 1);
evo_data(push, 0x0001);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 8cac8b724b70..407c91bd09b9 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -193,7 +193,23 @@ nv50_head_atomic_check_lut(struct nv50_head *head,
struct nv50_head_atom *asyh)
{
struct nv50_disp *disp = nv50_disp(head->base.base.dev);
+ struct drm_property_blob *ilut = asyh->state.degamma_lut;
struct drm_property_blob *olut = asyh->state.gamma_lut;
+ int ilut_size = ilut ? drm_color_lut_size(ilut) : 0;
+ int olut_size = olut ? drm_color_lut_size(olut) : 0;
+
+ if (!head->func->lut_chk(ilut_size)) {
+ DRM_DEBUG_KMS("Invalid DEGAMMA_LUT size: %d\n", ilut_size);
+ return -EINVAL;
+ }
+ if (!head->func->lut_chk(olut_size)) {
+ DRM_DEBUG_KMS("Invalid GAMMA_LUT size: %d\n", olut_size);
+ return -EINVAL;
+ }
+ if (ilut && olut && ilut_size != olut_size) {
+ DRM_DEBUG_KMS("DEGAMMA_LUT size (%d) must match GAMMA_LUT size
(%d)\n",
+ ilut_size, olut_size);
+ }
/* Determine whether core output LUT should be enabled. */
if (olut) {
@@ -217,7 +233,7 @@ nv50_head_atomic_check_lut(struct nv50_head *head,
asyh->olut.handle = disp->core->chan.vram.handle;
asyh->olut.buffer = !asyh->olut.buffer;
- head->func->olut(head, asyh);
+ head->func->olut(head, asyh, olut_size);
return 0;
}
@@ -491,12 +507,14 @@ nv50_head_create(struct drm_device *dev, int index)
drm_crtc_init_with_planes(dev, crtc, &wndw->plane, &curs->plane,
&nv50_head_func, "head-%d", head->base.index);
drm_crtc_helper_add(crtc, &nv50_head_help);
- drm_mode_crtc_set_gamma_size(crtc, 256);
+ drm_mode_crtc_set_gamma_size(crtc, head->func->lut_size)
Re: [PATCH] drm: Dump mode picture aspect ratio
On Thu, Jun 20, 2019 at 11:46 AM Ville Syrjala
wrote:
>
> From: Ville Syrjälä
>
> Currently the logs show nothing about the mode's aspect ratio.
> Include that information in the normal mode dump.
>
> Cc: Ilia Mirkin
> Signed-off-by: Ville Syrjälä
> ---
> drivers/video/hdmi.c| 3 ++-
> include/drm/drm_modes.h | 4 ++--
> include/linux/hdmi.h| 3 +++
> 3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index b939bc28d886..bc593fe1c268 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum
> hdmi_colorimetry colorimetry)
> return "Invalid";
> }
>
> -static const char *
> +const char *
> hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect)
> {
> switch (picture_aspect) {
> @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect
> picture_aspect)
> }
> return "Invalid";
> }
> +EXPORT_SYMBOL(hdmi_picture_aspect_get_name);
So this will return "No Data" most of the time (since the DRM_CAP
won't be enabled)? This will look awkward, esp since the person seeing
this print will have no idea what "No Data" is referring to.
>
> static const char *
> hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect)
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 083f16747369..2b1809c74fbe 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -431,7 +431,7 @@ struct drm_display_mode {
> /**
> * DRM_MODE_FMT - printf string for &struct drm_display_mode
> */
> -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x"
> +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s"
>
> /**
> * DRM_MODE_ARG - printf arguments for &struct drm_display_mode
> @@ -441,7 +441,7 @@ struct drm_display_mode {
> (m)->name, (m)->vrefresh, (m)->clock, \
> (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \
> (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \
> - (m)->type, (m)->flags
> + (m)->type, (m)->flags,
> hdmi_picture_aspect_get_name((m)->picture_aspect_ratio)
Flags are printed as a literal integer value. Given that AR stuff is
fairly esoteric, I might just print an integer here too. (Why was
picture_aspect_ratio not stuffed into ->flags in the first place?)
>
> #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
>
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 9918a6c910c5..de7cbe385dba 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame,
> void hdmi_infoframe_log(const char *level, struct device *dev,
> const union hdmi_infoframe *frame);
>
> +const char *
> +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect);
> +
> #endif /* _DRM_HDMI_H */
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC][PATCH] libdrm: modetest: Allow selecting modes by index
On Fri, Jun 21, 2019 at 6:07 PM John Stultz wrote:
>
> Often there are many similar modes, which cannot be selected
> via modetest due to its simple string matching.
>
> This change adds a mode index in the display output, which can
> then be used to specify a specific modeline to be set.
>
> Cc: Ilia Mirkin
> Cc: Rob Clark
> Cc: Bjorn Andersson
> Cc: Sumit Semwal
> Signed-off-by: John Stultz
> ---
> tests/modetest/modetest.c | 23 +--
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 9c85c07b..4cab5013 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -204,9 +204,10 @@ static void dump_encoders(struct device *dev)
> printf("\n");
> }
>
> -static void dump_mode(drmModeModeInfo *mode)
> +static void dump_mode(drmModeModeInfo *mode, int index)
> {
> - printf(" %s %d %d %d %d %d %d %d %d %d %d",
> + printf(" #%i %s %d %d %d %d %d %d %d %d %d %d",
> + index,
>mode->name,
>mode->vrefresh,
>mode->hdisplay,
> @@ -443,10 +444,10 @@ static void dump_connectors(struct device *dev)
>
> if (connector->count_modes) {
> printf(" modes:\n");
> - printf("\tname refresh (Hz) hdisp hss hse htot vdisp "
> + printf("\tindex name refresh (Hz) hdisp hss hse htot
> vdisp "
>"vss vse vtot)\n");
> for (j = 0; j < connector->count_modes; j++)
> - dump_mode(&connector->modes[j]);
> + dump_mode(&connector->modes[j], j);
> }
>
> if (_connector->props) {
> @@ -478,7 +479,7 @@ static void dump_crtcs(struct device *dev)
>crtc->buffer_id,
>crtc->x, crtc->y,
>crtc->width, crtc->height);
> - dump_mode(&crtc->mode);
> + dump_mode(&crtc->mode, 0);
>
> if (_crtc->props) {
> printf(" props:\n");
> @@ -829,6 +830,16 @@ connector_find_mode(struct device *dev, uint32_t con_id,
> const char *mode_str,
> if (!connector || !connector->count_modes)
> return NULL;
>
> + /* Pick by Index */
> + if (!strncmp(mode_str,"#",1)) {
if (mode_str[0] == '#') perhaps?
Otherwise this is
Reviewed-by: Ilia Mirkin
> + int index = atoi(mode_str + 1);
> +
> + if (index >= connector->count_modes)
> + return NULL;
> + return &connector->modes[index];
> + }
> +
> + /* Pick by Name */
> for (i = 0; i < connector->count_modes; i++) {
> mode = &connector->modes[i];
> if (!strcmp(mode->name, mode_str)) {
> @@ -1752,7 +1763,7 @@ static void usage(char *name)
>
> fprintf(stderr, "\n Test options:\n\n");
> fprintf(stderr, "\t-P
> @:x[++][*][@]\tset a plane\n");
> - fprintf(stderr, "\t-s
> [,][@]:[-][@]\tset
> a mode\n");
> + fprintf(stderr, "\t-s
> [,][@]:[# index>][-][@]\tset a mode\n");
> fprintf(stderr, "\t-C\ttest hw cursor\n");
> fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
> fprintf(stderr, "\t-w ::\tset property\n");
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] util: fix include path for drm_mode.h
Signed-off-by: Ilia Mirkin
---
tests/util/pattern.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/util/pattern.h b/tests/util/pattern.h
index 424b0e19..ea38cafd 100644
--- a/tests/util/pattern.h
+++ b/tests/util/pattern.h
@@ -26,7 +26,7 @@
#ifndef UTIL_PATTERN_H
#define UTIL_PATTERN_H
-#include
+#include
enum util_fill_pattern {
UTIL_PATTERN_TILES,
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] libdrm: modetest: Allow selecting modes by index
Reviewed-by: Ilia Mirkin
One minor comment below though:
(Maybe let it sit on the list for a day in case anyone feels like
objecting strenuously.)
On Mon, Jun 24, 2019 at 4:27 PM John Stultz wrote:
>
> Often there are many similar modes, which cannot be selected
> via modetest due to its simple string matching.
>
> This change adds a mode index in the display output, which can
> then be used to specify a specific modeline to be set.
>
> Cc: Ilia Mirkin
> Cc: Rob Clark
> Cc: Bjorn Andersson
> Cc: Sumit Semwal
> Signed-off-by: John Stultz
> ---
> v2: Reworked mode_str check per Ilia's suggestion
> ---
> tests/modetest/modetest.c | 23 +--
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index 9c85c07b..5a04190c 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -204,9 +204,10 @@ static void dump_encoders(struct device *dev)
> printf("\n");
> }
>
> -static void dump_mode(drmModeModeInfo *mode)
> +static void dump_mode(drmModeModeInfo *mode, int index)
> {
> - printf(" %s %d %d %d %d %d %d %d %d %d %d",
> + printf(" #%i %s %d %d %d %d %d %d %d %d %d %d",
> + index,
>mode->name,
>mode->vrefresh,
>mode->hdisplay,
> @@ -443,10 +444,10 @@ static void dump_connectors(struct device *dev)
>
> if (connector->count_modes) {
> printf(" modes:\n");
> - printf("\tname refresh (Hz) hdisp hss hse htot vdisp "
> + printf("\tindex name refresh (Hz) hdisp hss hse htot
> vdisp "
>"vss vse vtot)\n");
> for (j = 0; j < connector->count_modes; j++)
> - dump_mode(&connector->modes[j]);
> + dump_mode(&connector->modes[j], j);
> }
>
> if (_connector->props) {
> @@ -478,7 +479,7 @@ static void dump_crtcs(struct device *dev)
>crtc->buffer_id,
>crtc->x, crtc->y,
>crtc->width, crtc->height);
> - dump_mode(&crtc->mode);
> + dump_mode(&crtc->mode, 0);
>
> if (_crtc->props) {
> printf(" props:\n");
> @@ -829,6 +830,16 @@ connector_find_mode(struct device *dev, uint32_t con_id,
> const char *mode_str,
> if (!connector || !connector->count_modes)
> return NULL;
>
> + /* Pick by Index */
> + if (mode_str[0] == '#') {
> + int index = atoi(mode_str + 1);
> +
> + if (index >= connector->count_modes)
|| index < 0
or maybe just make it unsigned. Either way.
> + return NULL;
> + return &connector->modes[index];
> + }
> +
> + /* Pick by Name */
> for (i = 0; i < connector->count_modes; i++) {
> mode = &connector->modes[i];
> if (!strcmp(mode->name, mode_str)) {
> @@ -1752,7 +1763,7 @@ static void usage(char *name)
>
> fprintf(stderr, "\n Test options:\n\n");
> fprintf(stderr, "\t-P
> @:x[++][*][@]\tset a plane\n");
> - fprintf(stderr, "\t-s
> [,][@]:[-][@]\tset
> a mode\n");
> + fprintf(stderr, "\t-s
> [,][@]:[# index>][-][@]\tset a mode\n");
> fprintf(stderr, "\t-C\ttest hw cursor\n");
> fprintf(stderr, "\t-v\ttest vsynced page flipping\n");
> fprintf(stderr, "\t-w ::\tset property\n");
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm] tests/util: fix incorrect memset argument order
Make it actually clear the LUT.
Reported-by: Dave Airlie
Signed-off-by: Ilia Mirkin
---
tests/util/pattern.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/util/pattern.c b/tests/util/pattern.c
index 42a0e5c7..bf1797d4 100644
--- a/tests/util/pattern.c
+++ b/tests/util/pattern.c
@@ -643,7 +643,7 @@ void util_smpte_c8_gamma(unsigned size, struct
drm_color_lut *lut)
printf("Error: gamma too small: %d < %d\n", size, 7 + 7 + 8);
return;
}
- memset(lut, size * sizeof(struct drm_color_lut), 0);
+ memset(lut, 0, size * sizeof(struct drm_color_lut));
#define FILL_COLOR(idx, r, g, b) \
lut[idx].red = (r) << 8; \
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 20/22] mm: move hmm_vma_fault to nouveau
On Wed, Jul 3, 2019 at 1:49 PM Ralph Campbell wrote: > On 6/30/19 11:20 PM, Christoph Hellwig wrote: > > hmm_vma_fault is marked as a legacy API to get rid of, but quite suites > > the current nouvea flow. Move it to the only user in preparation for > > I didn't quite parse the phrase "quite suites the current nouvea flow." > s/nouvea/nouveau/ As long as you're fixing typos, suites -> suits. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/60] video: hdmi: Change return type of hdmi_avi_infoframe_init() to void
On Sun, Jul 7, 2019 at 2:15 PM Laurent Pinchart
wrote:
>
> Sorry, forgot to CC Bartlomiej on this patch.
>
> On Sun, Jul 07, 2019 at 09:07:54PM +0300, Laurent Pinchart wrote:
> > The hdmi_avi_infoframe_init() never needs to return an error, change its
> > return type to void.
> >
> > Signed-off-by: Laurent Pinchart
> > ---
> > drivers/gpu/drm/drm_edid.c | 5 +
> > drivers/video/hdmi.c | 9 ++---
> > include/linux/hdmi.h | 2 +-
> > 3 files changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> > index b939bc28d886..54fb7cf11d1a 100644
> > --- a/drivers/video/hdmi.c
> > +++ b/drivers/video/hdmi.c
> > @@ -56,15 +56,13 @@ static void hdmi_infoframe_set_checksum(void *buffer,
> > size_t size)
> > *
> > * Returns 0 on success or a negative error code on failure.
Probably want to adjust this text too, then?
[I have no opinion on whether this patch is good or bad, just happened
to notice the inconsistency.]
Cheers,
-ilia
> > */
> > -int hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame)
> > +void hdmi_avi_infoframe_init(struct hdmi_avi_infoframe *frame)
> > {
> > memset(frame, 0, sizeof(*frame));
> >
> > frame->type = HDMI_INFOFRAME_TYPE_AVI;
> > frame->version = 2;
> > frame->length = HDMI_AVI_INFOFRAME_SIZE;
> > -
> > - return 0;
> > }
> > EXPORT_SYMBOL(hdmi_avi_infoframe_init);
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] gpu/drm_memory: fix a few warnings
On Mon, Jul 8, 2019 at 2:06 PM Qian Cai wrote: > > The opening comment mark "/**" is reserved for kernel-doc comments, so > it will generate a warning with "make W=1". > > drivers/gpu/drm/drm_memory.c:2: warning: Cannot understand * \file > drm_memory.c > > Also, silence a checkpatch warning by adding a license identfiter where > it indicates the MIT license further down in the source file. > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > > It becomes redundant to add both an SPDX identifier and have a > description of the license in the comment block at the top, so remove > the later. > > Signed-off-by: Qian Cai > --- > > v2: remove the redundant description of the license. > > drivers/gpu/drm/drm_memory.c | 22 ++ > 1 file changed, 2 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/drm_memory.c b/drivers/gpu/drm/drm_memory.c > index 132fef8ff1b6..86a11fc8e954 100644 > --- a/drivers/gpu/drm/drm_memory.c > +++ b/drivers/gpu/drm/drm_memory.c > @@ -1,4 +1,5 @@ > -/** > +// SPDX-License-Identifier: MIT > +/* > * \file drm_memory.c > * Memory management wrappers for DRM > * > @@ -12,25 +13,6 @@ > * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas. > * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California. > * All Rights Reserved. > - * > - * Permission is hereby granted, free of charge, to any person obtaining a > - * copy of this software and associated documentation files (the "Software"), > - * to deal in the Software without restriction, including without limitation > - * the rights to use, copy, modify, merge, publish, distribute, sublicense, > - * and/or sell copies of the Software, and to permit persons to whom the > - * Software is furnished to do so, subject to the following conditions: > - * > - * The above copyright notice and this permission notice (including the next > - * paragraph) shall be included in all copies or substantial portions of the > - * Software. > - * > - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > - * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR This talks about VA Linux Systems and/or its suppliers, while the MIT licence talks about authors or copyright holders. Are such transformations OK to just do? -ilia > - * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > - * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > - * OTHER DEALINGS IN THE SOFTWARE. > */ > > #include > -- > 1.8.3.1 >
Re: [PATCH 2/2] drm/nouveau: Add HD-audio component notifier support
On Mon, Jul 22, 2019 at 10:38 AM Takashi Iwai wrote:
> +static void
> +nv50_audio_component_init(struct nouveau_drm *drm)
> +{
> + if (!component_add(drm->dev->dev, &nv50_audio_component_bind_ops))
> + drm->audio.component_registered = true;
> +}
Pardon my ignorance on this topic ... but are there any ill effects
from always doing this? For example, the board in question might not
have ELD support at all (the original G80, covered by dispnv50), or it
might not have any HDMI/DP ports, or it may have them but just nothing
is plugged in.
Could you confirm that this is all OK?
Thanks,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: My penguin has blue feets (aka: RGB versus BGR troubles)
On Fri, Jul 26, 2019 at 4:36 PM Sam Ravnborg wrote: > > Hi all. > > The Atmel at91sam9263 has a nice LCDC IP core that supports several > formats: > DRM_FORMAT_XBGR, DRM_FORMAT_BGR888, DRM_FORMAT_BGR565 > > (It also supports a palletized C8 format, but thats for later). > > The formats are all BGR formats - and some boards actually reverse Blue > and Red when wiring up the display. I have added a DT property to > identify boards with this difference. > > The penguin shown during boot of the kernel had blue feet which is a > clear sign that RED and GREEN was reversed. > > So to fix this I (got help from imirkin on irc) I implmented a quirk. > See patch below. > > With the quirk enabled I see: > - penguin shown during kernel boot has yellow feets (OK) > - modetest tell the driver reports: XB24 BG24 BG16 (as expected) > - modetest -s fails with: > # modetest -M atmel-lcdc -s 34:800x480-60Hz > setting mode 800x480-60Hz@XR24 on connectors 34, crtc 32 > failed to set mode: Function not implemented > > Snip from dmesg: > > drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_ADDFB2 > [drm:drm_mode_addfb2] [FB:37] > [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_SETCRTC > [drm:drm_mode_setcrtc] [CRTC:32:crtc-0] > [drm:drm_mode_setcrtc] Invalid pixel format XR24 little-endian (0x34325258), > modifier 0x0 > > [drm:drm_mode_object_put] OBJ ID: 37 (2) > [drm:drm_ioctl] pid=208, ret = -22 > [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_DIRTYFB > [drm:drm_mode_object_put] OBJ ID: 37 (2) > [drm:drm_ioctl] pid=208, ret = -38 > [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_RMFB > [drm:drm_mode_object_put] OBJ ID: 37 (2) > [drm:drm_mode_object_put] OBJ ID: 37 (1) > [drm:drm_ioctl] pid=208, dev=0xe201, auth=1, DRM_IOCTL_MODE_DESTROY_DUMB > [drm:drm_release] open_count = 1 > [drm:drm_file_free] pid = 208, device = 0xe201, open_count = 1 > [drm:drm_lastclose] > [drm:drm_lastclose] driver lastclose completed > > Notice that somehow we have a framebuffer in XR24 format, which is not > supported by the driver. > > > I have tried to tell that my driver supports DRM_FORMAT_XRGB, > DRM_FORMAT_RGB888, DRM_FORMAT_RGB565 and then modetest works. > But in the output of modetest -s the blue and red colors are reversed. > > *Any* hints why modetest fails when my driver reports the correct formats? Every driver to date supports XR24. So modetest uses that by default. But this fails with your driver since it's an unsupported format. Something like: modetest -M atmel-lcdc -s 34:800x480-60Hz@XB24 should do the trick. The quirk covers drivers that use AddFB(). However modetest is fancy and uses AddFB2, which takes an explicit format. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm/format-helper: Add drm_fb_{xrgb8888, gray8}_to_mono_reversed()
On Fri, Feb 4, 2022 at 10:53 AM Thomas Zimmermann wrote:
>
> Hi
>
> Am 04.02.22 um 14:43 schrieb Javier Martinez Canillas:
> > Add support to convert XR24 and 8-bit grayscale to reversed monochrome for
> > drivers that control monochromatic panels, that only have 1 bit per pixel.
> >
> > The drm_fb_gray8_to_mono_reversed() helper was based on the function that
> > does the same in the drivers/gpu/drm/tiny/repaper.c driver.
> >
> > Signed-off-by: Javier Martinez Canillas
> > ---
> >
> > (no changes since v1)
> >
> > drivers/gpu/drm/drm_format_helper.c | 80 +
> > include/drm/drm_format_helper.h | 7 +++
> > 2 files changed, 87 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_format_helper.c
> > b/drivers/gpu/drm/drm_format_helper.c
> > index 0f28dd2bdd72..cdce4b7c25d9 100644
> > --- a/drivers/gpu/drm/drm_format_helper.c
> > +++ b/drivers/gpu/drm/drm_format_helper.c
> > @@ -584,3 +584,83 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int
> > dst_pitch, uint32_t dst_for
> > return -EINVAL;
> > }
> > EXPORT_SYMBOL(drm_fb_blit_toio);
> > +
> > +static void drm_fb_gray8_to_mono_reversed_line(u8 *dst, const u8 *src,
> > size_t pixels)
> > +{
> > + unsigned int xb, i;
> > +
> > + for (xb = 0; xb < pixels / 8; xb++) {
>
> In practice, all mode widths are multiples of 8 because VGA mandated it.
> So it's ok-ish to assume this here. You should probably at least print a
> warning somewhere if (pixels % 8 != 0)
Not sure if it's relevant, but 1366x768 was a fairly popular laptop
resolution. There's even a dedicated drm_mode_fixup_1366x768 in
drm_edid.c. (Would it have killed them to add 2 more horizontal
pixels? Apparently.)
Cheers,
-ilia
Re: [PATCH v6 1/3] drm/lsdc: add drm driver for loongson display controller
On Wed, Feb 9, 2022 at 11:16 AM Maxime Ripard wrote:
>
> On Wed, Feb 09, 2022 at 10:38:41PM +0800, Sui Jingfeng wrote:
> > On 2022/2/9 16:49, Maxime Ripard wrote:
> > > On Fri, Feb 04, 2022 at 12:04:19AM +0800, Sui Jingfeng wrote:
> > > > > > +/* Get the simple EDID data from the device tree
> > > > > > + * the length must be EDID_LENGTH, since it is simple.
> > > > > > + *
> > > > > > + * @np: device node contain edid data
> > > > > > + * @edid_data: where the edid data to store to
> > > > > > + */
> > > > > > +static bool lsdc_get_edid_from_dtb(struct device_node *np,
> > > > > > +unsigned char *edid_data)
> > > > > > +{
> > > > > > + int length;
> > > > > > + const void *prop;
> > > > > > +
> > > > > > + if (np == NULL)
> > > > > > + return false;
> > > > > > +
> > > > > > + prop = of_get_property(np, "edid", &length);
> > > > > > + if (prop && (length == EDID_LENGTH)) {
> > > > > > + memcpy(edid_data, prop, EDID_LENGTH);
> > > > > > + return true;
> > > > > > + }
> > > > > > +
> > > > > > + return false;
> > > > > > +}
> > > > > You don't have a device tree binding for that driver, this is
> > > > > something
> > > > > that is required. And it's not clear to me why you'd want EDID in the
> > > > > DTB?
> > > > 1) It is left to the end user of this driver.
> > > >
> > > > The downstream motherboard maker may use a dpi(XRGB888) or LVDS panel
> > > > which don't have DDC support either, doing this way allow them put a
> > > > EDID property into the dc device node in the DTS. Then the entire
> > > > system works.
> > > > Note those panel usually support only one display mode.
> > > I guess it depends on what we mean exactly by the user, but the DTB
> > > usually isn't under the (end) user control. And the drm.edid_firmware is
> > > here already to address exactly this issue.
> > >
> > > On the other end, if the board has a static panel without any DDC lines,
> > > then just put the timings in the device tree, there's no need for an
> > > EDID blob.
> >
> > Loongson have a long history of using PMON firmware, The PMON firmware
> > support flush the dtb into the the firmware before grub loading the kernel.
> > You press 'c' key, then the PMON will give you a shell. it is much like a
> > UEFI shell. Suppose foo.dtb is what you want to pass to the vmlinuz.
> > Then type the follow single command can flush the dtb into the PMON
> > firmware.
> >
> > |load_dtb /dev/fs/fat@usb0/foo.dtb|
> >
> > For our PMON firmware, it**is** totally under developer/pc board maker's
> > control.
> > You can flush whatever dtb every time you bootup until you satisfied.
> > It(the pmon firmware) is designed to let downstream motherboard maker and/or
> > customers to play easily.
> >
> > Support of reading EDID from the dtb is really a feature which downstream
> > motherboard maker or customer wanted. They sometimes using eDP also whose
> > resolution is not 1024x768. This is out of control for a graphic driver
> > developer like me.
>
> And, to reinstate, we already have a mechanism to set an EDID, and if it
> wasn't an option, the DT is not the place to store an EDID blob.
Note that it's pretty common on laptop GPUs to have the attached
panel's EDID be baked into the VBIOS or ACPI (for LVDS / eDP). The hw
drivers in question (e.g. nouveau, radeon, probably i915) know how to
retrieve it specially. I'm no DT expert, but I'd imagine that it's
meant to allow mirroring those types of configurations. Stuff like
"drm.edid_firmware" isn't really meant for system-configuration-level
things, more as an out in case something doesn't work as it should.
Cheers,
-ilia
Re: [PATCH] drm/nouveau/bios: Use HWSQ entry 1 for PowerBook6,1
I'm not saying this is wrong, but could you file a bug at
gitlab.freedesktop.org/drm/nouveau/-/issues and include the VBIOS
(/sys/kernel/debug/dri/0/vbios.rom)? That would make it easier to
review the full situation.
On Mon, Feb 14, 2022 at 11:03 AM Icenowy Zheng wrote:
>
> On PowerBook6,1 (PowerBook G4 867 12") HWSQ entry 0 (which is currently
> always used by nouveau) fails, but the BIOS declares 2 HWSQ entries and
> entry 1 works.
>
> Add a quirk to use HWSQ entry 1.
>
> Signed-off-by: Icenowy Zheng
> ---
> drivers/gpu/drm/nouveau/nouveau_bios.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bios.c
> b/drivers/gpu/drm/nouveau/nouveau_bios.c
> index e8c445eb11004..2691d0e0cf9f1 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bios.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bios.c
> @@ -1977,6 +1977,13 @@ static int load_nv17_hw_sequencer_ucode(struct
> drm_device *dev,
> if (!hwsq_offset)
> return 0;
>
> +#ifdef __powerpc__
> + /* HWSQ entry 0 fails on PowerBook G4 867 12" (Al) */
> + if (of_machine_is_compatible("PowerBook6,1"))
> + return load_nv17_hwsq_ucode_entry(dev, bios,
> + hwsq_offset + sz, 1);
> +#endif
> +
> /* always use entry 0? */
> return load_nv17_hwsq_ucode_entry(dev, bios, hwsq_offset + sz, 0);
> }
> --
> 2.30.2
>
Re: [PATCH 1/2] drm: replace Compliance/Margin magic numbers with PCI_EXP_LNKCTL2 definitions
On Thu, Nov 7, 2019 at 5:21 PM Bjorn Helgaas wrote: > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h > index 29d6e93fd15e..03446be8a7be 100644 > --- a/include/uapi/linux/pci_regs.h > +++ b/include/uapi/linux/pci_regs.h > @@ -673,6 +673,8 @@ > #define PCI_EXP_LNKCTL2_TLS_8_0GT 0x0003 /* Supported Speed 8GT/s */ > #define PCI_EXP_LNKCTL2_TLS_16_0GT0x0004 /* Supported Speed 16GT/s */ > #define PCI_EXP_LNKCTL2_TLS_32_0GT0x0005 /* Supported Speed 32GT/s */ > +#define PCI_EXP_LNKCTL2_ENTER_COMP0x0010 /* Enter Compliance */ > +#define PCI_EXP_LNKCTL2_TX_MARGIN 0x0380 /* Enter Compliance */ Without commenting on the meat of the patch, this comment should probably read "Transmit Margin" or something along those lines? -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
On Tue, Nov 12, 2019 at 9:23 AM Böszörményi Zoltán wrote: > But no, all GPU devices (now only one, the UDL device) have screen 0 > (a.k.a. DISPLAY=:0.0) set when AutoBindGPU is true: > > [ 2444.576] xf86AutoConfigOutputDevices: xf86NumScreens 2 xf86NumGPUScreens 1 > [ 2444.576] xf86AutoConfigOutputDevices: GPU #0 driver 'modesetting' > 'modeset' scrnIndex > 256 origIndex 257 pScreen->myNum 256 confScreen->screennum 0 > confScreen->device->identifier 'Intel0' > confScreen->device->screen 0 confScreen->device->myScreenSection->screennum > 0 > confScreen->device->myScreenSection->device->screen 0 > > Somehow, Option "Device" should ensure that the UDL device is actually > treated as a framebuffer that can be rendered into (i.e. to be modeset(2) > instead of modeset(Gn)) and it should be woken up automatically. > > This is what AutoBindGPU is supposed to do, isn't it? > > But instead of assigning to screen 0, it should be assigned to whatever > screen number it is configured as. > > I know it's not a common use case nowadays, but I really want separate > fullscreen apps on their independent screens, including a standalone UDL > device, instead of having the latters as a Xinerama extension to some > other device. If you see a "G", that means it's being treated as a GPU device, which is *not* what you want if you want separate screens. You need to try to convince things to *not* set the devices up as GPU devices, but instead put each device (and each one of its heads, via ZaphodHeads) no a separate device, which in turn will have a separate screen. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
On Wed, Nov 13, 2019 at 11:59 AM Böszörményi Zoltán wrote: > > 2019. 11. 12. 17:41 keltezéssel, Ilia Mirkin írta: > > On Tue, Nov 12, 2019 at 9:23 AM Böszörményi Zoltán wrote: > >> But no, all GPU devices (now only one, the UDL device) have screen 0 > >> (a.k.a. DISPLAY=:0.0) set when AutoBindGPU is true: > >> > >> [ 2444.576] xf86AutoConfigOutputDevices: xf86NumScreens 2 > >> xf86NumGPUScreens 1 > >> [ 2444.576] xf86AutoConfigOutputDevices: GPU #0 driver 'modesetting' > >> 'modeset' scrnIndex > >> 256 origIndex 257 pScreen->myNum 256 confScreen->screennum 0 > >> confScreen->device->identifier 'Intel0' > >>confScreen->device->screen 0 > >> confScreen->device->myScreenSection->screennum 0 > >> confScreen->device->myScreenSection->device->screen 0 > >> > >> Somehow, Option "Device" should ensure that the UDL device is actually > >> treated as a framebuffer that can be rendered into (i.e. to be modeset(2) > >> instead of modeset(Gn)) and it should be woken up automatically. > >> > >> This is what AutoBindGPU is supposed to do, isn't it? > >> > >> But instead of assigning to screen 0, it should be assigned to whatever > >> screen number it is configured as. > >> > >> I know it's not a common use case nowadays, but I really want separate > >> fullscreen apps on their independent screens, including a standalone UDL > >> device, instead of having the latters as a Xinerama extension to some > >> other device. > > > > If you see a "G", that means it's being treated as a GPU device, which > > is *not* what you want if you want separate screens. You need to try > > to convince things to *not* set the devices up as GPU devices, but > > instead put each device (and each one of its heads, via ZaphodHeads) > > no a separate device, which in turn will have a separate screen. > > I created a merge request that finally made it possible what I wanted. > > https://gitlab.freedesktop.org/xorg/xserver/merge_requests/334 > > Now, no matter if I use the intel or modesetting drivers for the > Device sections using the Intel heads, or AutoBindGPU set to true or > false, the UDL device is correctly matched with its Option "kmsdev" > setting to the plaform device's device path. > > This patch seems to be a slight layering violation, but since the > modesetting driver is built into the Xorg server sources, the patch > may get away with it. Have you looked at setting AutoAddGPU to false? AutoBindGPU is too late -- that's when you already have a GPU, whether to bind it to the primary device (/screen/whatever). You need to not have a GPU in the first place. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 3/3] drm/nouveau: Support NVIDIA format modifiers
On Wed, Dec 11, 2019 at 4:04 PM James Jones wrote:
>
> Allow setting the block layout of a nouveau FB
> object using DRM format modifiers. When
> specified, the format modifier block layout and
> kind overrides the GEM buffer's implicit layout
> and kind. The specified format modifier is
> validated against he list of modifiers supported
> by the target display hardware.
>
> Signed-off-by: James Jones
> ---
> drivers/gpu/drm/nouveau/dispnv50/wndw.c | 8 +--
> drivers/gpu/drm/nouveau/nouveau_display.c | 65 ++-
> drivers/gpu/drm/nouveau/nouveau_display.h | 2 +
> 3 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> index 70ad64cb2d34..06c1b18479c1 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c
> @@ -43,7 +43,7 @@ nv50_wndw_ctxdma_new(struct nv50_wndw *wndw, struct
> nouveau_framebuffer *fb)
> {
> struct nouveau_drm *drm = nouveau_drm(fb->base.dev);
> struct nv50_wndw_ctxdma *ctxdma;
> - const u8kind = fb->nvbo->kind;
> + const u8kind = fb->kind;
> const u32 handle = 0xfb00 | kind;
> struct {
> struct nv_dma_v0 base;
> @@ -243,7 +243,7 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw,
> bool modeset,
> if (asyw->state.fb != armw->state.fb || !armw->visible || modeset) {
> asyw->image.w = fb->base.width;
> asyw->image.h = fb->base.height;
> - asyw->image.kind = fb->nvbo->kind;
> + asyw->image.kind = fb->kind;
>
> ret = nv50_wndw_atomic_check_acquire_rgb(asyw);
> if (ret) {
> @@ -255,9 +255,9 @@ nv50_wndw_atomic_check_acquire(struct nv50_wndw *wndw,
> bool modeset,
> if (asyw->image.kind) {
> asyw->image.layout = 0;
> if (drm->client.device.info.chipset >= 0xc0)
> - asyw->image.blockh = fb->nvbo->mode >> 4;
> + asyw->image.blockh = fb->tile_mode >> 4;
> else
> - asyw->image.blockh = fb->nvbo->mode;
> + asyw->image.blockh = fb->tile_mode;
> asyw->image.blocks[0] = fb->base.pitches[0] / 64;
> asyw->image.pitch[0] = 0;
> } else {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c
> b/drivers/gpu/drm/nouveau/nouveau_display.c
> index f1509392d7b7..351b58410e1a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_display.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_display.c
> @@ -224,6 +224,50 @@ static const struct drm_framebuffer_funcs
> nouveau_framebuffer_funcs = {
> .create_handle = nouveau_user_framebuffer_create_handle,
> };
>
> +static int
> +nouveau_decode_mod(struct nouveau_drm *drm,
> + uint64_t modifier,
> + uint32_t *tile_mode,
> + uint8_t *kind)
> +{
> + struct nouveau_display *disp = nouveau_display(drm->dev);
> + int mod;
> +
> + BUG_ON(!tile_mode || !kind);
> +
> + if (drm->client.device.info.chipset < 0x50) {
Not a full review, but you want to go off the family (chip_class iirc?
something like that, should be obvious). Sadly 0x67/0x68 are higher
than 0x50 numerically, but are logically part of the nv4x generation.
> + return -EINVAL;
> + }
> +
> + BUG_ON(!disp->format_modifiers);
> +
> + for (mod = 0;
> +(disp->format_modifiers[mod] != DRM_FORMAT_MOD_INVALID) &&
> +(disp->format_modifiers[mod] != modifier);
> +mod++);
> +
> + if (disp->format_modifiers[mod] == DRM_FORMAT_MOD_INVALID)
> + return -EINVAL;
> +
> + if (modifier == DRM_FORMAT_MOD_LINEAR) {
> + /* tile_mode will not be used in this case */
> + *tile_mode = 0;
> + *kind = 0;
> + } else {
> + /*
> +* Extract the block height and kind from the corresponding
> +* modifier fields. See drm_fourcc.h for details.
> +*/
> + *tile_mode = (uint32_t)(modifier & 0xF);
> + *kind = (uint8_t)((modifier >> 12) & 0xFF);
> +
> + if (drm->client.device.info.chipset >= 0xc0)
> + *tile_mode <<= 4;
> + }
> +
> + return 0;
> +}
> +
> static inline uint32_t
> nouveau_get_width_in_blocks(uint32_t stride)
> {
> @@ -300,6 +344,8 @@ nouveau_framebuffer_new(struct drm_device *dev,
> struct nouveau_framebuffer *fb;
> const struct drm_format_info *info;
> unsigned int width, height, i;
> + uint32_t tile_mode;
> + uint8_t kind;
> int ret;
>
> /* YUV overlays have special requirements pre-NV50 */
> @@ -322,6 +368,18 @@ nouvea
Re: [PATCH] drm/nouveau: declare constants as unsigned long.
Probably want ULL for 32-bit arches to be correct here too.
On Tue, Dec 31, 2019 at 3:53 PM Wambui Karuga wrote:
>
> Explicitly declare constants are unsigned long to address the following
> sparse warnings:
> warning: constant is so big it is long
>
> Signed-off-by: Wambui Karuga
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf108.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm107.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm200.c | 2 +-
> drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgp100.c | 2 +-
> 6 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c
> index ac87a3b6b7c9..506b358fcdb6 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c
> @@ -655,7 +655,7 @@ gf100_ram_new_(const struct nvkm_ram_func *func,
>
> static const struct nvkm_ram_func
> gf100_ram = {
> - .upper = 0x02,
> + .upper = 0x02UL,
> .probe_fbp = gf100_ram_probe_fbp,
> .probe_fbp_amount = gf100_ram_probe_fbp_amount,
> .probe_fbpa_amount = gf100_ram_probe_fbpa_amount,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf108.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf108.c
> index 70a06e3cd55a..3bc39895bbce 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf108.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf108.c
> @@ -43,7 +43,7 @@ gf108_ram_probe_fbp_amount(const struct nvkm_ram_func
> *func, u32 fbpao,
>
> static const struct nvkm_ram_func
> gf108_ram = {
> - .upper = 0x02,
> + .upper = 0x02UL,
> .probe_fbp = gf100_ram_probe_fbp,
> .probe_fbp_amount = gf108_ram_probe_fbp_amount,
> .probe_fbpa_amount = gf100_ram_probe_fbpa_amount,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
> index 456aed1f2a02..d01f32c0956a 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c
> @@ -1698,7 +1698,7 @@ gk104_ram_new_(const struct nvkm_ram_func *func, struct
> nvkm_fb *fb,
>
> static const struct nvkm_ram_func
> gk104_ram = {
> - .upper = 0x02,
> + .upper = 0x02UL,
> .probe_fbp = gf100_ram_probe_fbp,
> .probe_fbp_amount = gf108_ram_probe_fbp_amount,
> .probe_fbpa_amount = gf100_ram_probe_fbpa_amount,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm107.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm107.c
> index 27c68e3f9772..e24ac664eb15 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm107.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm107.c
> @@ -33,7 +33,7 @@ gm107_ram_probe_fbp(const struct nvkm_ram_func *func,
>
> static const struct nvkm_ram_func
> gm107_ram = {
> - .upper = 0x10,
> + .upper = 0x10UL,
> .probe_fbp = gm107_ram_probe_fbp,
> .probe_fbp_amount = gf108_ram_probe_fbp_amount,
> .probe_fbpa_amount = gf100_ram_probe_fbpa_amount,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm200.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm200.c
> index 6b0cac1fe7b4..17994cbda54b 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm200.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgm200.c
> @@ -48,7 +48,7 @@ gm200_ram_probe_fbp_amount(const struct nvkm_ram_func
> *func, u32 fbpao,
>
> static const struct nvkm_ram_func
> gm200_ram = {
> - .upper = 0x10,
> + .upper = 0x10UL,
> .probe_fbp = gm107_ram_probe_fbp,
> .probe_fbp_amount = gm200_ram_probe_fbp_amount,
> .probe_fbpa_amount = gf100_ram_probe_fbpa_amount,
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgp100.c
> b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgp100.c
> index adb62a6beb63..7a07a6ed4578 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgp100.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgp100.c
> @@ -79,7 +79,7 @@ gp100_ram_probe_fbpa(struct nvkm_device *device, int fbpa)
>
> static const struct nvkm_ram_func
> gp100_ram = {
> - .upper = 0x10,
> + .upper = 0x10UL,
> .probe_fbp = gm107_ram_probe_fbp,
> .probe_fbp_amount = gm200_ram_probe_fbp_amount,
> .probe_fbpa_amount = gp100_ram_probe_fbpa,
> --
> 2.17.1
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 2/3] drm/rockchip: Add optional support for CRTC gamma LUT
On Thu, Oct 10, 2019 at 12:01 PM Sean Paul wrote:
> > > > +static int vop_crtc_atomic_check(struct drm_crtc *crtc,
> > > > + struct drm_crtc_state *crtc_state)
> > > > +{
> > > > + struct vop *vop = to_vop(crtc);
> > > > +
> > > > + if (vop->lut_regs && crtc_state->color_mgmt_changed &&
> > > > + crtc_state->gamma_lut) {
> > > > + unsigned int len;
> > > > +
> > > > + len = drm_color_lut_size(crtc_state->gamma_lut);
> > > > + if (len != crtc->gamma_size) {
> > > > + DRM_DEBUG_KMS("Invalid LUT size; got %d, expected
> > > > %d\n",
> > > > + len, crtc->gamma_size);
> > > > + return -EINVAL;
> > > > + }
> > >
> > > Overflow is avoided in drm_mode_gamma_set_ioctl(), so I don't think you
> > > need
> > > this function.
> > >
> >
> > But that only applies to the legacy path. Isn't this needed to ensure
> > a gamma blob
> > has the right size?
>
> Yeah, good point, we check the element size in the atomic path, but not the
> max
> size. I haven't looked at enough color lut stuff to have an opinion whether
> this
> check would be useful in a helper function or not, something to consider, I
> suppose.
Some implementations support multiple sizes (e.g. 256 and 1024) but
not anything in between. It would be difficult to expose this
generically, I would imagine. The 256 size is kind of special, since
basically all legacy usage assumes that 256 is the one true quantity
of LUT entries...
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()
On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
wrote:
>
> Add a new helper function drm_color_ctm_s31_32_to_qm_n() for driver to
> convert S31.32 sign-magnitude to Qm.n 2's complement that supported by
> hardware.
>
> Signed-off-by: james qian wang (Arm Technology China)
>
> ---
> drivers/gpu/drm/drm_color_mgmt.c | 23 +++
> include/drm/drm_color_mgmt.h | 2 ++
> 2 files changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..3d533d0b45af 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -132,6 +132,29 @@ uint32_t drm_color_lut_extract(uint32_t user_input,
> uint32_t bit_precision)
> }
> EXPORT_SYMBOL(drm_color_lut_extract);
>
> +/**
> + * drm_color_ctm_s31_32_to_qm_n
> + *
> + * @user_input: input value
> + * @m: number of integer bits
Is this the full 2's complement value? i.e. including the "sign" bit
of the 2's complement representation? I'd kinda assume that m = 32, n
= 0 would just get me the integer portion of this, for example.
> + * @n: number of fractinal bits
fractional
> + *
> + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> + */
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> + uint32_t m, uint32_t n)
> +{
> + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> + bool negative = !!(user_input & BIT_ULL(63));
> + s64 val;
> +
> + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
This implies that n = 32, m = 0 would actually yield a 33-bit 2's
complement number. Is that what you meant?
> + val = clamp_val(mag, 0, negative ? BIT(n + m) : BIT(n + m) - 1);
I'm going to play with numpy to convince myself that this is right
(esp with the endpoints), but in the meanwhile, you probably want to
use BIT_ULL in case n + m > 32 (I don't think that's the case with any
current hardware though).
> +
> + return negative ? 0ll - val : val;
Why not just "negative ? -val : val"?
> +}
> +EXPORT_SYMBOL(drm_color_ctm_s31_32_to_qm_n);
> +
> /**
> * drm_crtc_enable_color_mgmt - enable color management properties
> * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..60fea5501886 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -30,6 +30,8 @@ struct drm_crtc;
> struct drm_plane;
>
> uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> + uint32_t m, uint32_t n);
>
> void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> uint degamma_lut_size,
> --
> 2.20.1
>
Re: [PATCH v2 1/4] drm/komeda: Add a new helper drm_color_ctm_s31_32_to_qm_n()
On Mon, Oct 14, 2019 at 9:16 PM james qian wang (Arm Technology China)
wrote:
> On Mon, Oct 14, 2019 at 11:58:48AM -0400, Ilia Mirkin wrote:
> > On Fri, Oct 11, 2019 at 1:43 AM james qian wang (Arm Technology China)
> > wrote:
> > > + *
> > > + * Convert and clamp S31.32 sign-magnitude to Qm.n 2's complement.
> > > + */
> > > +uint64_t drm_color_ctm_s31_32_to_qm_n(uint64_t user_input,
> > > + uint32_t m, uint32_t n)
> > > +{
> > > + u64 mag = (user_input & ~BIT_ULL(63)) >> (32 - n);
> > > + bool negative = !!(user_input & BIT_ULL(63));
> > > + s64 val;
> > > +
> > > + /* the range of signed 2s complement is [-2^n+m, 2^n+m - 1] */
> >
> > This implies that n = 32, m = 0 would actually yield a 33-bit 2's
> > complement number. Is that what you meant?
>
> Yes, since m doesn't include sign-bit So a Q0.32 is a 33bit value.
This goes counter to what the wikipedia page says [
https://en.wikipedia.org/wiki/Q_(number_format) ]:
(reformatted slightly for text-only consumption):
"""
For example, a Q15.1 format number:
- requires 15+1 = 16 bits
- its range is [-2^14, 2^14 - 2^-1] = [-16384.0, +16383.5] = [0x8000,
0x8001 ... 0x, 0x, 0x0001 ... 0x7FFE, 0x7FFF]
- its resolution is 2^-1 = 0.5
"""
This suggests that the proper way to represent a standard 32-bit 2's
complement integer would be Q32.0.
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
On Tue, Oct 22, 2019 at 11:50 AM Böszörményi Zoltán wrote: > > Hi, > > I have the below configuration for an Intel based POS system that, > while advertises 3 outputs (DP1, VGA1 and HDMI1 with xf86-video-intel), > only two are usable. DP1 for the built-in touchscreen and VGA1 for > the external VGA connector. > > I wanted to use an USB DisplayLink device as the 3rd output, with all > three output using its own Screen number, i.e. :0.0 :0.1 and :0.2. > > [...] > > How can I set up 3 different Screens correctly for 3 separate fullscreen > applications? > > I am using Xorg 1.20.4 patched with the "autobind GPUs to the screen" > patch from Dave Airlie that at least wakes up the UDL device and makes > it visible without extra magic with providers/sinks. If it's being treated as a GPU, that's your first problem for this kind of setup. You should see modeset(2), in your logs, but I suspect you're seeing modeset(G0) (the "G" indicates "GPU"). > > # cat /etc/X11/xorg.conf.d/videocard.conf > Section "Monitor" > Identifier "Monitor-DP-1" > Option "AutoServerLayout" "on" > Option "Rotate" "normal" > EndSection > > Section "Monitor" > Identifier "Monitor-HDMI-1" > Option "AutoServerLayout" "on" > Option "Rotate" "normal" > EndSection > > Section "Monitor" > Identifier "Monitor-VGA-1" > Option "AutoServerLayout" "on" > Option "Rotate" "normal" > EndSection > > Section "Monitor" > Identifier "DVI-I-1-1" The others are Monitor-*, this one isn't. You probably want this to be DVI-I-1, as noted below. I guess you get the extra -1 from seeing it as a slaved GPU's output in your current configuration. > Option "AutoServerLayout" "on" > Option "Rotate" "normal" > EndSection > > Section "Device" > Identifier "Intel0" > Driver "modesetting" > Option "kmsdev" "/dev/dri/card1" > Screen 0 > Option "Monitor-DP1" "DP-1" > Option "ZaphodHeads" "DP-1" > EndSection > > Section "Device" > Identifier "Intel1" > Driver "modesetting" > Option "kmsdev" "/dev/dri/card1" > Screen 1 > Option "Monitor-VGA-1" "VGA-1" > Option "ZaphodHeads" "VGA-1" > EndSection > > # Intentionally not referenced in ServerLayout below > Section "Device" > Identifier "Intel2" > Driver "modesetting" > Option "kmsdev" "/dev/dri/card1" > Option "Monitor-HDMI-1" "HDMI-1" > Option "ZaphodHeads" "HDMI-1" > EndSection > > Section "Device" > Identifier "UDL" > Driver "modesetting" > Option "kmsdev" "/dev/dri/card0" > Screen 2 > Option "Monitor-DVI-I-1-1" "DVI-I-1-1" I think you have an extra -1 in here (and the monitor name doesn't exist as per above). And I think the "Screen" index is wrong -- it's not what one tends to think it is, as I recall. I think you can just drop these lines though. > EndSection > > Section "Screen" > Identifier "SCREEN" > Option "AutoServerLayout" "on" > Device "Intel0" > Monitor "Monitor-DP-1" > SubSection "Display" > Modes "1024x768" > Depth 24 > EndSubSection > EndSection > > Section "Screen" > Identifier "SCREEN1" > Option "AutoServerLayout" "on" > Device "Intel1" > Monitor "Monitor-VGA-1" > SubSection "Display" > Modes "1024x768" > Depth 24 > EndSubSection > EndSection > > Section "Screen" > Identifier "SCREEN2" > Option "AutoServerLayout" "on" > Device "UDL" > Monitor "Monitor-DVI-I-1-1" > SubSection "Display" > Modes "1024x768" > Depth 24 > EndSubSection > EndSection > > Section "ServerLayout" > Identifier "LAYOUT" > Option "AutoServerLayout" "on" > Screen 0 "SCREEN" > Screen 1 "SCREEN1" RightOf "SCREEN" > Screen 2 "SCREEN2" RightOf "SCREEN1" > EndSection > > Best regards, > Zoltán Böszörményi > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: UDL device cannot get its own screen
On Wed, Oct 23, 2019 at 2:41 AM Böszörményi Zoltán wrote: > > 2019. 10. 22. 22:57 keltezéssel, Ilia Mirkin írta: > > On Tue, Oct 22, 2019 at 11:50 AM Böszörményi Zoltán wrote: > >> Section "Device" > >> Identifier "UDL" > >> Driver "modesetting" > >> Option "kmsdev" "/dev/dri/card0" > >> Screen 2 > >> Option "Monitor-DVI-I-1-1" "DVI-I-1-1" > > > > I think you have an extra -1 in here (and the monitor name doesn't > > exist as per above). And I think the "Screen" index is wrong -- it's > > not what one tends to think it is, as I recall. I think you can just > > drop these lines though. > > Without "Screen N" lines, all the outputs are assigned to :0 > so the screen layout setup in the ServerLayout section is not > applied properly. > As I remember it, the Screen here is for ZaphodHeads-type configurations, and it indicates which head you're supposed to use of the underlying device. My suggestion was to only remove it here, not everywhere. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] nv50_disp_chan_mthd: ensure mthd is not NULL
Hi Frédéric,
It appears Ben made his own version of this patch (probably based on
the one you added to the kernel bz), and it's already upstream:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.6-rc2&id=0e6176c6d286316e9431b4f695940cfac4ffe6c2
Cheers,
-ilia
On Thu, Feb 20, 2020 at 12:19 PM Frédéric Pierret
wrote:
>
> Hi,
> Is anything missing here? How can I get this merged?
>
> Best regards,
> Frédéric Pierret
>
> On 2020-02-08 20:43, Frédéric Pierret wrote:
> > Pointer to structure array is assumed not NULL by default. It has
> > the consequence to raise a kernel panic when it's not the case.
> >
> > Basically, running at least a RTX2080TI on Xen makes a bad mmio error
> > which causes having 'mthd' pointer to be NULL in 'channv50.c'. From the
> > code, it's assumed to be not NULL by accessing directly 'mthd->data[0]'
> > which is the reason of the kernel panic. Simply check if the pointer
> > is not NULL before continuing.
> >
> > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=206299
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Frédéric Pierret (fepitre)
> > ---
> > drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c
> > b/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c
> > index bcf32d92ee5a..50e3539f33d2 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/channv50.c
> > @@ -74,6 +74,8 @@ nv50_disp_chan_mthd(struct nv50_disp_chan *chan, int
> > debug)
> >
> > if (debug > subdev->debug)
> > return;
> > + if (!mthd)
> > + return;
> >
> > for (i = 0; (list = mthd->data[i].mthd) != NULL; i++) {
> > u32 base = chan->head * mthd->addr;
> >
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH 1/3] drm: Add separate state structure for legacy, non-KMS drivers
On Tue, Feb 25, 2020 at 10:59 AM Thomas Zimmermann wrote:
>
> Non-KMS drivers store state in struct drm_driver. This bloats the
> structure for KMS drivers and prevents it from being declared with
> 'static const' qualifiers. Moving the non-KMS state into a separate
> data structure resolves this.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_drv.c | 4
> drivers/gpu/drm/i810/i810_drv.c | 4
> drivers/gpu/drm/mga/mga_drv.c | 4
> drivers/gpu/drm/nouveau/nouveau_drm.c | 8
> drivers/gpu/drm/r128/r128_drv.c | 4
> drivers/gpu/drm/savage/savage_drv.c | 4
> drivers/gpu/drm/sis/sis_drv.c | 4
> drivers/gpu/drm/tdfx/tdfx_drv.c | 4
> drivers/gpu/drm/via/via_drv.c | 4
> include/drm/drm_drv.h | 3 +++
> include/drm/drm_legacy.h | 6 ++
> 11 files changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 7b1a628d1f6e..4ba0df097602 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -625,6 +625,10 @@ int drm_dev_init(struct drm_device *dev,
> if (WARN_ON(!parent))
> return -EINVAL;
>
> + if (drm_core_check_feature(dev, DRIVER_LEGACY) &&
> + WARN_ON(!driver->legacy))
> + return -EINVAL;
> +
> kref_init(&dev->ref);
> dev->dev = get_device(parent);
> dev->driver = driver;
> diff --git a/drivers/gpu/drm/i810/i810_drv.c b/drivers/gpu/drm/i810/i810_drv.c
> index 0e53a066d4db..55f17f00bae9 100644
> --- a/drivers/gpu/drm/i810/i810_drv.c
> +++ b/drivers/gpu/drm/i810/i810_drv.c
> @@ -56,6 +56,9 @@ static const struct file_operations i810_driver_fops = {
> .llseek = noop_llseek,
> };
>
> +static struct drm_legacy_state = i810_legacy_state {
Does this compile? I might have assumed this would need to be
static struct drm_legacy_state i810_legacy_state = {
> +};
> +
> static struct drm_driver driver = {
> .driver_features = DRIVER_USE_AGP | DRIVER_HAVE_DMA | DRIVER_LEGACY,
> .dev_priv_size = sizeof(drm_i810_buf_priv_t),
> @@ -71,6 +74,7 @@ static struct drm_driver driver = {
> .major = DRIVER_MAJOR,
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
> + .legacy = &i810_legacy_state,
> };
>
> static struct pci_driver i810_pci_driver = {
> diff --git a/drivers/gpu/drm/mga/mga_drv.c b/drivers/gpu/drm/mga/mga_drv.c
> index 71128e6f6ae9..4865982d949c 100644
> --- a/drivers/gpu/drm/mga/mga_drv.c
> +++ b/drivers/gpu/drm/mga/mga_drv.c
> @@ -53,6 +53,9 @@ static const struct file_operations mga_driver_fops = {
> .llseek = noop_llseek,
> };
>
> +static struct drm_legacy_state = mga_legacy_state {
> +};
> +
> static struct drm_driver driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_LEGACY |
> @@ -78,6 +81,7 @@ static struct drm_driver driver = {
> .major = DRIVER_MAJOR,
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
> + .legacy = &mga_legacy_state,
> };
>
> static struct pci_driver mga_pci_driver = {
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 6b1629c14dd7..c88cf32e521c 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1103,6 +1103,11 @@ nouveau_driver_fops = {
> .llseek = noop_llseek,
> };
>
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> +static struct drm_legacy_state nouveau_legacy_state{
> +};
> +#endif
> +
> static struct drm_driver
> driver_stub = {
> .driver_features =
> @@ -1150,6 +1155,9 @@ driver_stub = {
> .major = DRIVER_MAJOR,
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
> +#if defined(CONFIG_NOUVEAU_LEGACY_CTX_SUPPORT)
> + .legacy = &nouveau_legacy_state,
> +#endif
> };
>
> static struct pci_device_id
> diff --git a/drivers/gpu/drm/r128/r128_drv.c b/drivers/gpu/drm/r128/r128_drv.c
> index b7a5f162ebae..ecece3a48d93 100644
> --- a/drivers/gpu/drm/r128/r128_drv.c
> +++ b/drivers/gpu/drm/r128/r128_drv.c
> @@ -57,6 +57,9 @@ static const struct file_operations r128_driver_fops = {
> .llseek = noop_llseek,
> };
>
> +static struct drm_legacy_state = r128_legacy_state {
> +};
> +
> static struct drm_driver driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_PCI_DMA | DRIVER_SG | DRIVER_LEGACY |
> @@ -81,6 +84,7 @@ static struct drm_driver driver = {
> .major = DRIVER_MAJOR,
> .minor = DRIVER_MINOR,
> .patchlevel = DRIVER_PATCHLEVEL,
> + .legacy = &r128_legacy_state,
> };
>
> int r128_driver_load(struct drm_device *dev, unsigned long flags)
> diff --git a/drivers/gpu/drm/savage/savage_drv.c
> b/drivers/gpu/drm/savage/savage_drv.c
> index 799bd11adb9c..c0a7146fbde1 100644
> --- a/drivers/gpu/drm/savage/savage_
Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true
Pretty sure the current code is right. If the GAMMA_LUT property can't
be set, it tries to set gamma the old-fashioned way.
On Tue, Mar 3, 2020 at 8:12 AM Devarsh Thakkar wrote:
>
> Hi Rohit,
>
> This makes sense to me as gamma was implemented as optional property.
> Reviewed-By: "Devarsh Thakkar "
>
> @emil.veli...@collabora.com, @imir...@alum.mit.edu, @Ville Syrjälä, Could you
> please ack and help merge this patch if it also look good to you ?
>
> Regards,
> Devarsh
>
> > -Original Message-
> > From: Rohit Visavalia
> > Sent: 27 February 2020 00:40
> > To: Rohit Visavalia ; dri-devel@lists.freedesktop.org;
> > imir...@alum.mit.edu; emil.veli...@collabora.com
> > Cc: Hyun Kwon ; Ranganathan Sk ; Dhaval
> > Rajeshbhai Shah ; Varunkumar Allagadapa
> > ; Devarsh Thakkar
> > Subject: RE: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if
> > add_property_optional returns true
> >
> > Gentle reminder.
> >
> > + Ilia Mirkin, +Emil Velikov.
> >
> > Thanks & Regards,
> > Rohit
> >
> > > -Original Message-
> > > From: Rohit Visavalia [mailto:rohit.visava...@xilinx.com]
> > > Sent: Tuesday, February 25, 2020 3:08 PM
> > > To: dri-devel@lists.freedesktop.org
> > > Cc: Hyun Kwon ; Ranganathan Sk ;
> > > Dhaval Rajeshbhai Shah ; Varunkumar Allagadapa
> > > ; Devarsh Thakkar ; Rohit
> > > Visavalia
> > > Subject: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if
> > > add_property_optional returns true
> > >
> > > gamma is a optional property then also it prints error message, so set
> > > gamma only if add_property_optional() returns true.
> > >
> > > Signed-off-by: Rohit Visavalia
> > > ---
> > > tests/modetest/modetest.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > index b907ab3..379b9ea 100644
> > > --- a/tests/modetest/modetest.c
> > > +++ b/tests/modetest/modetest.c
> > > @@ -1138,7 +1138,7 @@ static void set_gamma(struct device *dev,
> > > unsigned crtc_id, unsigned fourcc)
> > >
> > > add_property_optional(dev, crtc_id, "DEGAMMA_LUT", 0);
> > > add_property_optional(dev, crtc_id, "CTM", 0);
> > > - if (!add_property_optional(dev, crtc_id, "GAMMA_LUT", blob_id)) {
> > > + if (add_property_optional(dev, crtc_id, "GAMMA_LUT", blob_id)) {
> > > uint16_t r[256], g[256], b[256];
> > >
> > > for (i = 0; i < 256; i++) {
> > > --
> > > 2.7.4
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: avoid double-attaching hdmi/edp bridges
Each of hdmi and edp are already attached in msm_*_bridge_init. A second attachment returns -EBUSY, failing the driver load. Tested with HDMI on IFC6410 (APQ8064 / MDP4), but eDP case should be analogous. Fixes: 3ef2f119bd3ed (drm/msm: Use drm_attach_bridge() to attach a bridge to an encoder) Cc: Boris Brezillon Signed-off-by: Ilia Mirkin --- drivers/gpu/drm/msm/edp/edp.c | 4 drivers/gpu/drm/msm/hdmi/hdmi.c | 4 2 files changed, 8 deletions(-) diff --git a/drivers/gpu/drm/msm/edp/edp.c b/drivers/gpu/drm/msm/edp/edp.c index ad4e963ccd9b..106a67473af5 100644 --- a/drivers/gpu/drm/msm/edp/edp.c +++ b/drivers/gpu/drm/msm/edp/edp.c @@ -178,10 +178,6 @@ int msm_edp_modeset_init(struct msm_edp *edp, struct drm_device *dev, goto fail; } - ret = drm_bridge_attach(encoder, edp->bridge, NULL); - if (ret) - goto fail; - priv->bridges[priv->num_bridges++] = edp->bridge; priv->connectors[priv->num_connectors++] = edp->connector; diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 1a9b6289637d..737453b6e596 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -327,10 +327,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, goto fail; } - ret = drm_bridge_attach(encoder, hdmi->bridge, NULL); - if (ret) - goto fail; - priv->bridges[priv->num_bridges++] = hdmi->bridge; priv->connectors[priv->num_connectors++] = hdmi->connector; -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true
Hm. I'm not sure offhand how to check if drmModeCrtcSetGamma is supported.
I guess you could check if gamma size > 0 or something?
On Thu, Mar 12, 2020, 02:39 Rohit Visavalia wrote:
> Hi Ilia Mirkin,
>
> Thanks for the review.
>
> By old-fashioned way you mean to say using drmModeCrtcSetGamma()? If yes
> then, it shows error as "failed to set gamma: Function no implemented" if
> any platform specific drm has no gamma property implemented.
>
> Current code shows error while running modetest for Xilinx drm as it
> doesn't supports gamma property and ideally it should not show error as
> gamma is optional property, so it doesn't serve the purpose of optional
> property.
>
> Please correct me if I am missing anything.
>
> Thanks
> Rohit
>
> > -Original Message-
> > From: Ilia Mirkin [mailto:imir...@alum.mit.edu]
> > Sent: Tuesday, March 3, 2020 7:08 PM
> > To: Devarsh Thakkar
> > Cc: Rohit Visavalia ;
> dri-devel@lists.freedesktop.org;
> > emil.veli...@collabora.com; Ville Syrjälä ;
> Hyun
> > Kwon ; Ranganathan Sk ; Dhaval
> > Rajeshbhai Shah ; Varunkumar Allagadapa
> >
> > Subject: Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if
> > add_property_optional returns true
> >
> > EXTERNAL EMAIL
> >
> > Pretty sure the current code is right. If the GAMMA_LUT property can't
> be set,
> > it tries to set gamma the old-fashioned way.
> >
> > On Tue, Mar 3, 2020 at 8:12 AM Devarsh Thakkar
> > wrote:
> > >
> > > Hi Rohit,
> > >
> > > This makes sense to me as gamma was implemented as optional property.
> > > Reviewed-By: "Devarsh Thakkar "
> > >
> > > @emil.veli...@collabora.com, @imir...@alum.mit.edu, @Ville Syrjälä,
> > Could you please ack and help merge this patch if it also look good to
> you ?
> > >
> > > Regards,
> > > Devarsh
> > >
> > > > -Original Message-
> > > > From: Rohit Visavalia
> > > > Sent: 27 February 2020 00:40
> > > > To: Rohit Visavalia ;
> > > > dri-devel@lists.freedesktop.org; imir...@alum.mit.edu;
> > > > emil.veli...@collabora.com
> > > > Cc: Hyun Kwon ; Ranganathan Sk ;
> > > > Dhaval Rajeshbhai Shah ; Varunkumar Allagadapa
> > > > ; Devarsh Thakkar
> > > > Subject: RE: [PATCH libdrm] modetest: call drmModeCrtcSetGamma()
> > > > only if add_property_optional returns true
> > > >
> > > > Gentle reminder.
> > > >
> > > > + Ilia Mirkin, +Emil Velikov.
> > > >
> > > > Thanks & Regards,
> > > > Rohit
> > > >
> > > > > -Original Message-
> > > > > From: Rohit Visavalia [mailto:rohit.visava...@xilinx.com]
> > > > > Sent: Tuesday, February 25, 2020 3:08 PM
> > > > > To: dri-devel@lists.freedesktop.org
> > > > > Cc: Hyun Kwon ; Ranganathan Sk ;
> > > > > Dhaval Rajeshbhai Shah ; Varunkumar Allagadapa
> > > > > ; Devarsh Thakkar ;
> > > > > Rohit Visavalia
> > > > > Subject: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only
> > > > > if add_property_optional returns true
> > > > >
> > > > > gamma is a optional property then also it prints error message, so
> > > > > set gamma only if add_property_optional() returns true.
> > > > >
> > > > > Signed-off-by: Rohit Visavalia
> > > > > ---
> > > > > tests/modetest/modetest.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> > > > > index b907ab3..379b9ea 100644
> > > > > --- a/tests/modetest/modetest.c
> > > > > +++ b/tests/modetest/modetest.c
> > > > > @@ -1138,7 +1138,7 @@ static void set_gamma(struct device *dev,
> > > > > unsigned crtc_id, unsigned fourcc)
> > > > >
> > > > > add_property_optional(dev, crtc_id, "DEGAMMA_LUT", 0);
> > > > > add_property_optional(dev, crtc_id, "CTM", 0);
> > > > > - if (!add_property_optional(dev, crtc_id, "GAMMA_LUT",
> blob_id)) {
> > > > > + if (add_property_optional(dev, crtc_id, "GAMMA_LUT", blob_id))
> > > > > + {
> > > > > uint16_t r[256], g[256], b[256];
> > > > >
> > > > > for (i = 0; i < 256; i++) {
> > > > > --
> > > > > 2.7.4
> > >
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if add_property_optional returns true
It's to restore the gamma ramp to be the default linear one. Let's say that the driver doesn't have the GAMMA_LUT property support, and then you want to modeset with C8 (indexed) format. That means that modeset has to set the LUT to make the indexed lookups work (which it does, it all works, you celebrate). Then you want to run modeset with e.g. XR24, and the colors are all black! The LUT is persistent across modesets, so it has to reset the ramp to linear. You could condition calling set_gamma on crtc->gamma_size > 0, I think -- it didn't occur to me that there would be drivers that didn't support a LUT. On Fri, Mar 13, 2020 at 6:08 AM Rohit Visavalia wrote: > > Hi Ilia Mirkin, > > > > But it should not go for setting gamma(drmModeCrtcSetGamma) as user has not > asked to do so in just simple mode set command(modetest -M -s > 42:3840x2160@RG16). > > > > What is the requirement for setting gamma drmModeCrtcSetGamma() if user has > not asked ? > > > > Thanks > Rohit > > > > From: Ilia Mirkin [mailto:imir...@alum.mit.edu] > Sent: Thursday, March 12, 2020 3:49 PM > To: Rohit Visavalia > Cc: Devarsh Thakkar ; dri-devel > ; emil.veli...@collabora.com; Ville Syrjälä > ; Hyun Kwon ; Ranganathan Sk > ; Dhaval Rajeshbhai Shah ; Varunkumar > Allagadapa > Subject: Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if > add_property_optional returns true > > > > CAUTION: This message has originated from an External Source. Please use > proper judgment and caution when opening attachments, clicking links, or > responding to this email. > > > > Hm. I'm not sure offhand how to check if drmModeCrtcSetGamma is supported. I > guess you could check if gamma size > 0 or something? > > > > On Thu, Mar 12, 2020, 02:39 Rohit Visavalia wrote: > > Hi Ilia Mirkin, > > Thanks for the review. > > By old-fashioned way you mean to say using drmModeCrtcSetGamma()? If yes > then, it shows error as "failed to set gamma: Function no implemented" if any > platform specific drm has no gamma property implemented. > > Current code shows error while running modetest for Xilinx drm as it doesn't > supports gamma property and ideally it should not show error as gamma is > optional property, so it doesn't serve the purpose of optional property. > > Please correct me if I am missing anything. > > Thanks > Rohit > > > -Original Message- > > From: Ilia Mirkin [mailto:imir...@alum.mit.edu] > > Sent: Tuesday, March 3, 2020 7:08 PM > > To: Devarsh Thakkar > > Cc: Rohit Visavalia ; dri-devel@lists.freedesktop.org; > > emil.veli...@collabora.com; Ville Syrjälä ; > > Hyun > > Kwon ; Ranganathan Sk ; Dhaval > > Rajeshbhai Shah ; Varunkumar Allagadapa > > > > Subject: Re: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() only if > > add_property_optional returns true > > > > EXTERNAL EMAIL > > > > Pretty sure the current code is right. If the GAMMA_LUT property can't be > > set, > > it tries to set gamma the old-fashioned way. > > > > On Tue, Mar 3, 2020 at 8:12 AM Devarsh Thakkar > > wrote: > > > > > > Hi Rohit, > > > > > > This makes sense to me as gamma was implemented as optional property. > > > Reviewed-By: "Devarsh Thakkar " > > > > > > @emil.veli...@collabora.com, @imir...@alum.mit.edu, @Ville Syrjälä, > > Could you please ack and help merge this patch if it also look good to you ? > > > > > > Regards, > > > Devarsh > > > > > > > -Original Message- > > > > From: Rohit Visavalia > > > > Sent: 27 February 2020 00:40 > > > > To: Rohit Visavalia ; > > > > dri-devel@lists.freedesktop.org; imir...@alum.mit.edu; > > > > emil.veli...@collabora.com > > > > Cc: Hyun Kwon ; Ranganathan Sk ; > > > > Dhaval Rajeshbhai Shah ; Varunkumar Allagadapa > > > > ; Devarsh Thakkar > > > > Subject: RE: [PATCH libdrm] modetest: call drmModeCrtcSetGamma() > > > > only if add_property_optional returns true > > > > > > > > Gentle reminder. > > > > > > > > + Ilia Mirkin, +Emil Velikov. > > > > > > > > Thanks & Regards, > > > > Rohit > > > > > > > > > -Original Message- > > > > > From: Rohit Visavalia [mailto:rohit.visava...@xilinx.com] > > > > > Sent: Tuesday, February 25, 2020 3:08 PM > > > > > To: dri-devel@lists.freedesktop.org > > &
Re: [PATCH 4.19 092/190] drm/nouveau: Dont WARN_ON VCPI allocation failures
Hi Greg,
This feels like it's missing a From: line.
commit b513a18cf1d705bd04efd91c417e79e4938be093
Author: Lyude Paul
Date: Mon Jan 28 16:03:50 2019 -0500
drm/nouveau: Don't WARN_ON VCPI allocation failures
Is this an artifact of your notification-of-patches process and I
never noticed before, or was the patch ingested incorrectly?
Cheers,
-ilia
On Fri, Sep 13, 2019 at 9:16 AM Greg Kroah-Hartman
wrote:
>
> [ Upstream commit b513a18cf1d705bd04efd91c417e79e4938be093 ]
>
> This is much louder then we want. VCPI allocation failures are quite
> normal, since they will happen if any part of the modesetting process is
> interrupted by removing the DP MST topology in question. So just print a
> debugging message on VCPI failures instead.
>
> Signed-off-by: Lyude Paul
> Fixes: f479c0ba4a17 ("drm/nouveau/kms/nv50: initial support for DP 1.2
> multi-stream")
> Cc: Ben Skeggs
> Cc: dri-devel@lists.freedesktop.org
> Cc: nouv...@lists.freedesktop.org
> Cc: # v4.10+
> Signed-off-by: Ben Skeggs
> Signed-off-by: Sasha Levin
> ---
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index f889d41a281fa..5e01bfb69d7a3 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -759,7 +759,8 @@ nv50_msto_enable(struct drm_encoder *encoder)
>
> slots = drm_dp_find_vcpi_slots(&mstm->mgr, mstc->pbn);
> r = drm_dp_mst_allocate_vcpi(&mstm->mgr, mstc->port, mstc->pbn,
> slots);
> - WARN_ON(!r);
> + if (!r)
> + DRM_DEBUG_KMS("Failed to allocate VCPI\n");
>
> if (!mstm->links++)
> nv50_outp_acquire(mstm->outp);
> --
> 2.20.1
>
>
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4.19 092/190] drm/nouveau: Dont WARN_ON VCPI allocation failures
On Fri, Sep 13, 2019 at 10:46 AM Sasha Levin wrote: > > On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: > >Hi Greg, > > > >This feels like it's missing a From: line. > > > >commit b513a18cf1d705bd04efd91c417e79e4938be093 > >Author: Lyude Paul > >Date: Mon Jan 28 16:03:50 2019 -0500 > > > >drm/nouveau: Don't WARN_ON VCPI allocation failures > > > >Is this an artifact of your notification-of-patches process and I > >never noticed before, or was the patch ingested incorrectly? > > It was always like this for patches that came through me. Greg's script > generates an explicit "From:" line in the patch, but I never saw the > value in that since git does the right thing by looking at the "From:" > line in the mail header. That's right -- the email's From header gets used in the case of no explicit From in the email body. But Greg is sending the emails From: Greg, so if I were to ingest that email, I would end up with a patch From: Greg, not From: Lyude as it ought to be. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4.19 092/190] drm/nouveau: Dont WARN_ON VCPI allocation failures
On Fri, Sep 13, 2019 at 11:01 AM Sasha Levin wrote: > > On Fri, Sep 13, 2019 at 03:54:56PM +0100, Greg Kroah-Hartman wrote: > >On Fri, Sep 13, 2019 at 10:46:27AM -0400, Sasha Levin wrote: > >> On Fri, Sep 13, 2019 at 09:33:36AM -0400, Ilia Mirkin wrote: > >> > Hi Greg, > >> > > >> > This feels like it's missing a From: line. > >> > > >> > commit b513a18cf1d705bd04efd91c417e79e4938be093 > >> > Author: Lyude Paul > >> > Date: Mon Jan 28 16:03:50 2019 -0500 > >> > > >> >drm/nouveau: Don't WARN_ON VCPI allocation failures > >> > > >> > Is this an artifact of your notification-of-patches process and I > >> > never noticed before, or was the patch ingested incorrectly? > >> > >> It was always like this for patches that came through me. Greg's script > >> generates an explicit "From:" line in the patch, but I never saw the > >> value in that since git does the right thing by looking at the "From:" > >> line in the mail header. > >> > >> The right thing is being done in stable-rc and for the releases. For > >> your example here, this is how it looks like in the stable-rc tree: > >> > >> commit bdcc885be68289a37d0d063cd94390da81fd8178 > >> Author: Lyude Paul > >> AuthorDate: Mon Jan 28 16:03:50 2019 -0500 > >> Commit: Greg Kroah-Hartman > >> CommitDate: Fri Sep 13 14:05:29 2019 +0100 > >> > >>drm/nouveau: Don't WARN_ON VCPI allocation failures > > > >Yeah, we should fix your scripts to put the explicit From: line in here > >as we are dealing with patches in this format and it causes confusion at > >times (like now.) It's not the first time and that's why I added those > >lines to the patches. > > Heh, didn't think anyone cared about this scenario for the stable-rc > patches. > > I'll go add it. > > But... why do you actually care? Just a hygiene thing. Everyone else sends patches the normal way, with accurate attribution. Why should stable be different? (I was surprised to see Greg contributing to nouveau when I first saw the patch. But then realized it was the stable ingestion notification.) -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/nouveau: dispnv50: Remove nv50_mstc_best_encoder()
On Fri, Sep 13, 2019 at 6:05 PM Lyude Paul wrote:
>
> When drm_connector_helper_funcs->atomic_best_encoder is defined,
> ->best_encoder is ignored both by the atomic modesetting helpers. That
By both the atomic modesetting helpers and ... (usually "both" implies 2 things)
> being said, this hook is completely broken anyway - it always returns
> the first msto for a given mstc, despite the fact it might already be in
> use.
>
> So, just get rid of it. We'll need this in a moment anyway, when we make
> mstos per-head as opposed to per-connector.
>
> Signed-off-by: Lyude Paul
> ---
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 -
> 1 file changed, 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index b46be8a091e9..a3f350fdfa8c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -920,14 +920,6 @@ nv50_mstc_atomic_best_encoder(struct drm_connector
> *connector,
> return &mstc->mstm->msto[head->base.index]->encoder;
> }
>
> -static struct drm_encoder *
> -nv50_mstc_best_encoder(struct drm_connector *connector)
> -{
> - struct nv50_mstc *mstc = nv50_mstc(connector);
> -
> - return &mstc->mstm->msto[0]->encoder;
> -}
> -
> static enum drm_mode_status
> nv50_mstc_mode_valid(struct drm_connector *connector,
> struct drm_display_mode *mode)
> @@ -990,7 +982,6 @@ static const struct drm_connector_helper_funcs
> nv50_mstc_help = {
> .get_modes = nv50_mstc_get_modes,
> .mode_valid = nv50_mstc_mode_valid,
> - .best_encoder = nv50_mstc_best_encoder,
> .atomic_best_encoder = nv50_mstc_atomic_best_encoder,
> .atomic_check = nv50_mstc_atomic_check,
> };
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/36] drm/nouveau: use bpp instead of cpp for drm_format_info
On Mon, Sep 23, 2019 at 8:56 AM Sandy Huang wrote:
>
> cpp[BytePerPlane] can't describe the 10bit data format correctly,
> So we use bpp[BitPerPlane] to instead cpp.
>
> Signed-off-by: Sandy Huang
> ---
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 7 ---
> drivers/gpu/drm/nouveau/dispnv50/base507c.c | 4 ++--
> drivers/gpu/drm/nouveau/dispnv50/ovly507e.c | 2 +-
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index f22f010..59d2f07 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -874,11 +874,12 @@ nv04_crtc_do_mode_set_base(struct drm_crtc *crtc,
>
> /* Update the framebuffer location. */
> regp->fb_start = nv_crtc->fb.offset & ~3;
> - regp->fb_start += (y * drm_fb->pitches[0]) + (x *
> drm_fb->format->cpp[0]);
> + regp->fb_start += (y * drm_fb->pitches[0]) +
> + (x * drm_fb->format->bpp[0] / 8);
> nv_set_crtc_base(dev, nv_crtc->index, regp->fb_start);
>
> /* Update the arbitration parameters. */
> - nouveau_calc_arb(dev, crtc->mode.clock, drm_fb->format->cpp[0] * 8,
> + nouveau_calc_arb(dev, crtc->mode.clock, drm_fb->format->bpp[0],
> &arb_burst, &arb_lwm);
>
> regp->CRTC[NV_CIO_CRE_FF_INDEX] = arb_burst;
> @@ -1238,7 +1239,7 @@ nv04_crtc_page_flip(struct drm_crtc *crtc, struct
> drm_framebuffer *fb,
>
> /* Initialize a page flip struct */
> *s = (struct nv04_page_flip_state)
> - { { }, event, crtc, fb->format->cpp[0] * 8, fb->pitches[0],
> + { { }, event, crtc, fb->format->bpp[0], fb->pitches[0],
> new_bo->bo.offset };
>
> /* Keep vblanks on during flip, for the target crtc of this flip */
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> index d5e295c..59883bd0 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/base507c.c
> @@ -190,12 +190,12 @@ base507c_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw,
> return ret;
>
> if (!wndw->func->ilut) {
> - if ((asyh->base.cpp != 1) ^ (fb->format->cpp[0] != 1))
> + if (asyh->base.cpp != 1 ^ fb->format->bpp[0] != 8)
Please leave the parens in. Even if it works out to the same thing
(don't know), ^ vs != ordering isn't fresh in many people's minds
(mine included).
> asyh->state.color_mgmt_changed = true;
> }
>
> asyh->base.depth = fb->format->depth;
> - asyh->base.cpp = fb->format->cpp[0];
> + asyh->base.cpp = fb->format->bpp[0] / 8;
> asyh->base.x = asyw->state.src.x1 >> 16;
> asyh->base.y = asyw->state.src.y1 >> 16;
> asyh->base.w = asyw->state.fb->width;
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> b/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> index cc41766..c6c2e0b 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/ovly507e.c
> @@ -135,7 +135,7 @@ ovly507e_acquire(struct nv50_wndw *wndw, struct
> nv50_wndw_atom *asyw,
> if (ret)
> return ret;
>
> - asyh->ovly.cpp = fb->format->cpp[0];
> + asyh->ovly.cpp = fb->format->bpp[0] / 8;
> return 0;
> }
>
> --
> 2.7.4
>
>
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] Is Nouveau really using the io_reserve_lru?
On Thu, Sep 26, 2019 at 5:44 PM Ben Skeggs wrote: > > On Tue, 24 Sep 2019 at 22:19, Christian König > wrote: > > > > Hi guys, > > > > while working through more old TTM functionality I stumbled over the > > io_reserve_lru. > > > > Basic idea is that when this flag is set the driver->io_mem_reserve() > > callback can return -EAGAIN resulting in unmapping of other BOs. > > > > But Nouveau doesn't seem to return -EAGAIN in the call path of > > io_mem_reserve anywhere. > I believe this is a bug in Nouveau. We *should* be returning -EAGAIN > if we fail to find space in BAR1 to map the BO into. Could this lead to SIGBUS in userspace, esp related to resume and similar situations? A user has been experiencing this in a tricky-to-reproduce scenario with a ton of vram dedicated to framebuffers and so on (3x 4K), and the nouveau ddx falls back to memcpy in certain cases. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/komeda: Add drm_ctm_to_coeffs()
On Mon, Sep 30, 2019 at 7:05 AM Brian Starkey wrote:
>
> Hi James,
>
> On Mon, Sep 30, 2019 at 04:54:41AM +, james qian wang (Arm Technology
> China) wrote:
> > This function is used to convert drm_color_ctm S31.32 sign-magnitude
> > value to komeda required Q2.12 2's complement
> >
> > Signed-off-by: james qian wang (Arm Technology China)
> >
> > ---
> > .../arm/display/komeda/komeda_color_mgmt.c| 27 +++
> > .../arm/display/komeda/komeda_color_mgmt.h| 1 +
> > 2 files changed, 28 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
> > b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
> > index c180ce70c26c..c92c82eebddb 100644
> > --- a/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
> > +++ b/drivers/gpu/drm/arm/display/komeda/komeda_color_mgmt.c
> > @@ -117,3 +117,30 @@ void drm_lut_to_fgamma_coeffs(struct drm_property_blob
> > *lut_blob, u32 *coeffs)
> > {
> > drm_lut_to_coeffs(lut_blob, coeffs, sector_tbl,
> > ARRAY_SIZE(sector_tbl));
> > }
> > +
> > +/* Convert from S31.32 sign-magnitude to HW Q2.12 2's complement */
> > +static s32 drm_ctm_s31_32_to_q2_12(u64 input)
> > +{
> > + u64 mag = (input & ~BIT_ULL(63)) >> 20;
> > + bool negative = !!(input & BIT_ULL(63));
> > + u32 val;
> > +
> > + /* the range of signed 2s complement is [-2^n, 2^n - 1] */
> > + val = clamp_val(mag, 0, negative ? BIT(14) : BIT(14) - 1);
> > +
> > + return negative ? 0 - val : val;
> > +}
>
> This function looks generally useful. Should it be in DRM core
> (assuming there isn't already one there)?
>
> You can use a parameter to determine the number of bits desired in the
> output.
I suspect every driver needs to do something similar. You can see what
I did for nouveau here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=88b703527ba70659365d989f29579f1292ebf9c3
(look for csc_drm_to_base)
Would be great to have a common helper which correctly accounts for
all the variability.
Cheers,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Display-Port HPD handling, link status, and bandwidth checks
You'll probably get a more detailed reply during the week, but for now have a look at the "link-status" property, which was made for precisely this situation. I think basically the idea is to ignore link training as part of the modeset, and just return the link status depending on the success. (And you should filter out totally infeasible modes, i.e. outside the monitor's max lanes/bandwidth capabilities, which I believe are available via DPCD or EDID.) See https://www.kernel.org/doc/html/latest/gpu/drm-kms.html for a bit more info as well. Cheers, -ilia On Sun, Aug 25, 2019 at 7:12 AM Jyri Sarha wrote: > > Hi, > > I am working on a new DisplayPort bridge-driver and there is a couple of > things that I do not know how to handle. > > 1. When should the link training happen? >a) In connector detect()? > - This would enable us to do mode filtering (in mode_valid()) > based on the established link band-width (then again > mode_valid() documentation suggests that modes should only > be filtered based on "configuration-invariant hardware > constraints"). >b) In check phase (this would currently mean mode_fixup)? > - This is the last point where we can reject a mode that can not > be sent over the DP-link >c) In commit phase (e.g. bridge enable()) > - This is bad since we should not fail any more in the commit > phase > > 2. DP-link sometimes drops after a succesful link training and DP-sink >is supposed to send short HPD pulse about it. What are the >recommended ways to handle the situation? > >a) Send hotplug event and let the DRM client deal with it? > - This does not work too well because even if the client tries > to restore the display by committing the same state again - > like fbdev does - the bridge does not go trough disable-enable > cycle, since display mode has not changed. > - Despite it not working so well, this is what the most drivers > appear to do. > >b) Driver internally re-trains the link but send a hotplug event > always after it? > - This is what i915 does, if I read the code right. > - How to treat a training failure? Sending hotplug event does not > really help (see above). > >c) Silently re-train the link if we were able to restore the link > and the display mode, and send HPD only if something went wrong? > > Best regards, > Jyri > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH v3 5/7] drm/nouveau: utilize subconnector property for DP
This should probably be fixed to account for the scenario where an
HDMI connector is plugged directly into the DP++ port. I don't think
the dp.subconnector property will be valid in that case.
(Unfortunately I don't remember how one detects that particular
situation.)
On Mon, Aug 26, 2019 at 9:22 AM Oleg Vasilev wrote:
>
> Since DP-specific information is stored in driver's structures, every
> driver needs to implement subconnector property by itself.
>
> Reviewed-by: Emil Velikov
> Signed-off-by: Oleg Vasilev
> Cc: Ben Skeggs
> Cc: nouv...@lists.freedesktop.org
> ---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 13 +
> drivers/gpu/drm/nouveau/nouveau_dp.c| 9 +
> drivers/gpu/drm/nouveau/nouveau_encoder.h | 1 +
> 3 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 94dfa2e5a9ab..d9c116cc11b9 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -635,6 +635,17 @@ nouveau_connector_detect(struct drm_connector
> *connector, bool force)
> pm_runtime_mark_last_busy(dev->dev);
> pm_runtime_put_autosuspend(dev->dev);
>
> + if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> + connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> + enum drm_mode_subconnector subconnector =
> DRM_MODE_SUBCONNECTOR_Unknown;
> +
> + if (conn_status == connector_status_connected && nv_encoder)
> + subconnector = nv_encoder->dp.subconnector;
> + drm_object_property_set_value(&connector->base,
> + connector->dev->mode_config.dp_subconnector_property,
> + subconnector);
> + }
> +
> return conn_status;
> }
>
> @@ -1359,6 +1370,8 @@ nouveau_connector_create(struct drm_device *dev,
> kfree(nv_connector);
> return ERR_PTR(ret);
> }
> +
> + drm_mode_add_dp_subconnector_property(connector);
> funcs = &nouveau_connector_funcs;
> break;
> default:
> diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
> b/drivers/gpu/drm/nouveau/nouveau_dp.c
> index 2674f1587457..85eac853e3f8 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dp.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
> @@ -62,6 +62,7 @@ nouveau_dp_detect(struct nouveau_encoder *nv_encoder)
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct nvkm_i2c_aux *aux;
> u8 dpcd[8];
> + u8 port_cap[DP_MAX_DOWNSTREAM_PORTS] = {};
> int ret;
>
> aux = nv_encoder->aux;
> @@ -72,6 +73,14 @@ nouveau_dp_detect(struct nouveau_encoder *nv_encoder)
> if (ret)
> return ret;
>
> + if (dpcd[DP_DPCD_REV] > 0x10) {
> + ret = nvkm_rdaux(aux, DP_DOWNSTREAM_PORT_0,
> +port_cap, DP_MAX_DOWNSTREAM_PORTS);
> + if (ret)
> + memset(port_cap, 0, DP_MAX_DOWNSTREAM_PORTS);
> + }
> + nv_encoder->dp.subconnector = drm_dp_subconnector_type(dpcd,
> port_cap);
> +
> nv_encoder->dp.link_bw = 27000 * dpcd[1];
> nv_encoder->dp.link_nr = dpcd[2] & DP_MAX_LANE_COUNT_MASK;
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_encoder.h
> b/drivers/gpu/drm/nouveau/nouveau_encoder.h
> index 3517f920bf89..e17971a30221 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_encoder.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_encoder.h
> @@ -63,6 +63,7 @@ struct nouveau_encoder {
> struct nv50_mstm *mstm;
> int link_nr;
> int link_bw;
> + enum drm_mode_subconnector subconnector;
> } dp;
> };
>
> --
> 2.23.0
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
