Re: [PATCH 1/6] drm/nouveau: convert to using is_hdmi and has_audio from display info
On Fri, Jan 12, 2024 at 11:50 AM Jani Nikula wrote:
>
> Prefer the parsed results for is_hdmi and has_audio in display info over
> calling drm_detect_hdmi_monitor() and drm_detect_monitor_audio(),
> respectively.
>
> Conveniently, this also removes the need to use edid_blob_ptr.
>
> Cc: Karol Herbst
> Cc: Lyude Paul
> Cc: Danilo Krummrich
> Cc: nouv...@lists.freedesktop.org
> Signed-off-by: Jani Nikula
> ---
> drivers/gpu/drm/nouveau/dispnv50/disp.c | 8
> drivers/gpu/drm/nouveau/dispnv50/head.c | 8 +---
> drivers/gpu/drm/nouveau/nouveau_connector.c | 2 +-
> 3 files changed, 6 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 8d37a694b772..908b1042669c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -750,7 +750,7 @@ nv50_audio_enable(struct drm_encoder *encoder, struct
> nouveau_crtc *nv_crtc,
> struct nouveau_encoder *nv_encoder = nouveau_encoder(encoder);
> struct nvif_outp *outp = _encoder->outp;
>
> - if (!nv50_audio_supported(encoder) ||
> !drm_detect_monitor_audio(nv_connector->edid))
> + if (!nv50_audio_supported(encoder) ||
> !nv_connector->base.display_info.has_audio)
> return;
>
> mutex_lock(>audio.lock);
> @@ -1764,7 +1764,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder,
> struct drm_atomic_state *sta
> if ((disp->disp->object.oclass == GT214_DISP ||
> disp->disp->object.oclass >= GF110_DISP) &&
> nv_encoder->dcb->type != DCB_OUTPUT_LVDS &&
> - drm_detect_monitor_audio(nv_connector->edid))
> + nv_connector->base.display_info.has_audio)
> hda = true;
>
> if (!nvif_outp_acquired(outp))
> @@ -1773,7 +1773,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder,
> struct drm_atomic_state *sta
> switch (nv_encoder->dcb->type) {
> case DCB_OUTPUT_TMDS:
> if (disp->disp->object.oclass != NV50_DISP &&
> - drm_detect_hdmi_monitor(nv_connector->edid))
> + !nv_connector->base.display_info.is_hdmi)
This is backwards, no?
> nv50_hdmi_enable(encoder, nv_crtc, nv_connector,
> state, mode, hda);
>
> if (nv_encoder->outp.or.link & 1) {
> @@ -1786,7 +1786,7 @@ nv50_sor_atomic_enable(struct drm_encoder *encoder,
> struct drm_atomic_state *sta
> */
> if (mode->clock >= 165000 &&
> nv_encoder->dcb->duallink_possible &&
> - !drm_detect_hdmi_monitor(nv_connector->edid))
> + !nv_connector->base.display_info.is_hdmi)
> proto =
> NV507D_SOR_SET_CONTROL_PROTOCOL_DUAL_TMDS;
> } else {
> proto = NV507D_SOR_SET_CONTROL_PROTOCOL_SINGLE_TMDS_B;
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
> b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 83355dbc15ee..d7c74cc43ba5 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -127,14 +127,8 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
> struct drm_display_mode *omode = >state.adjusted_mode;
> struct drm_display_mode *umode = >state.mode;
> int mode = asyc->scaler.mode;
> - struct edid *edid;
> int umode_vdisplay, omode_hdisplay, omode_vdisplay;
>
> - if (connector->edid_blob_ptr)
> - edid = (struct edid *)connector->edid_blob_ptr->data;
> - else
> - edid = NULL;
> -
> if (!asyc->scaler.full) {
> if (mode == DRM_MODE_SCALE_NONE)
> omode = umode;
> @@ -162,7 +156,7 @@ nv50_head_atomic_check_view(struct nv50_head_atom *armh,
> */
> if ((asyc->scaler.underscan.mode == UNDERSCAN_ON ||
> (asyc->scaler.underscan.mode == UNDERSCAN_AUTO &&
> -drm_detect_hdmi_monitor(edid {
> +connector->display_info.is_hdmi))) {
> u32 bX = asyc->scaler.underscan.hborder;
> u32 bY = asyc->scaler.underscan.vborder;
> u32 r = (asyh->view.oH << 19) / asyh->view.oW;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 856b3ef5edb8..938832a6af15 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -1034,7 +1034,7 @@ get_tmds_link_bandwidth(struct drm_connector *connector)
> unsigned duallink_scale =
> nouveau_duallink && nv_encoder->dcb->duallink_possible ? 2 :
> 1;
>
> - if (drm_detect_hdmi_monitor(nv_connector->edid)) {
> + if (nv_connector->base.display_info.is_hdmi) {
> info =
Re: [Nouveau] [PATCH v2] drm/nouveau: bring back blit subchannel for pre nv50 GPUs
On Fri, May 26, 2023 at 5:11 AM Karol Herbst wrote:
>
> 1ba6113a90a0 removed a lot of the kernel GPU channel, but method 0x128
> was important as otherwise the GPU spams us with `CACHE_ERROR` messages.
>
> We use the blit subchannel inside our vblank handling, so we should keep
> at least this part.
>
> v2: Only do it for NV11+ GPUs
>
> Closes: https://gitlab.freedesktop.org/drm/nouveau/-/issues/201
> Fixes: 4a16dd9d18a0 ("drm/nouveau/kms: switch to drm fbdev helpers")
> Signed-off-by: Karol Herbst
> ---
> drivers/gpu/drm/nouveau/nouveau_chan.c | 1 +
> drivers/gpu/drm/nouveau/nouveau_chan.h | 1 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 20 +---
> 3 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c
> b/drivers/gpu/drm/nouveau/nouveau_chan.c
> index e648ecd0c1a0..3dfbc374478e 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> @@ -90,6 +90,7 @@ nouveau_channel_del(struct nouveau_channel **pchan)
> if (cli)
> nouveau_svmm_part(chan->vmm->svmm, chan->inst);
>
> + nvif_object_dtor(>blit);
> nvif_object_dtor(>nvsw);
> nvif_object_dtor(>gart);
> nvif_object_dtor(>vram);
> diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.h
> b/drivers/gpu/drm/nouveau/nouveau_chan.h
> index e06a8ffed31a..bad7466bd0d5 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_chan.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_chan.h
> @@ -53,6 +53,7 @@ struct nouveau_channel {
> u32 user_put;
>
> struct nvif_object user;
> + struct nvif_object blit;
>
> struct nvif_event kill;
> atomic_t killed;
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index cc7c5b4a05fd..9512f1c2f871 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -369,15 +369,29 @@ nouveau_accel_gr_init(struct nouveau_drm *drm)
> ret = nvif_object_ctor(>channel->user, "drmNvsw",
>NVDRM_NVSW, nouveau_abi16_swclass(drm),
>NULL, 0, >channel->nvsw);
> +
> + if (ret == 0 && device->info.chipset >= 0x11) {
Can you double-check that this is needed on NV15? IIRC there's some
non-linearity of chipsets here which is why we had (some long time
ago, not sure if it's still there), a chip class which would simplify
such checks.
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 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 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] 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 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", );
> > > > > > + 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 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] 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 = >mode_config;
>
> if (degamma_lut_size) {
> + crtc->degamma_lut_size = degamma_lut_size;
> drm_object_attach_property(>base,
>
Re: [Nouveau] Trouble with TTM patches w/nouveau in linux-next
GEM init happens here: https://cgit.freedesktop.org/drm/drm/tree/drivers/gpu/drm/nouveau/nouveau_gem.c#n221 Note the bo alloc / gem init / bo init dance. I don't think there is a GEM object for internal allocations at all -- we just allocate bo's directly and that's it. Perhaps you meant something else? I thought GEM was meant for externally-available objects. Cheers, -ilia On Wed, Jun 9, 2021 at 10:58 AM Christian König wrote: > > Good point, but I think that is unrelated. > > My suspicion is rather that nouveau is not initializing the underlying > GEM object for internal allocations. > > So what happens is the same as on VMWGFX that TTM doesn't know anything > about the size to of the BO resulting in a kmalloc() with a random value > and eventually -ENOMEM. > > Good news is that I can reproduce it, so going to look into that later > today. > > Regards, > Christian. > > Am 09.06.21 um 16:52 schrieb Ilia Mirkin: > > Christian - potentially relevant is that Tegra doesn't have VRAM at > > all -- all GTT (or GART or whatever it's called nowadays). No > > fake/stolen VRAM. > > > > Cheers, > > > >-ilia > > > > On Wed, Jun 9, 2021 at 10:18 AM Christian König > > wrote: > >> Hi Mikko, > >> > >> strange sounds like Nouveau was somehow also using the GEM workaround > >> for VMWGFX as well. > >> > >> But -12 means -ENOMEM which doesn't fits into the picture. > >> > >> I will try with a G710, but if that doesn't yields anything I need some > >> more input from you. > >> > >> Thanks for the report, > >> Christian. > >> > >> > >> Am 09.06.21 um 15:47 schrieb Mikko Perttunen: > >>> Hi, > >>> > >>> I'm observing nouveau not initializing recently on linux-next on my > >>> Tegra186 Jetson TX2 board. Specifically it looks like BO allocation is > >>> failing when initializing the sync subsystem: > >>> > >>> [ 21.858149] nouveau 1700.gpu: DRM: failed to initialise sync > >>> subsystem, -28 > >>> > >>> I have been bisecting and I have found two patches that affect this. > >>> Firstly, things first break on > >>> > >>> d02117f8efaa drm/ttm: remove special handling for non GEM drivers > >>> > >>> starting to return error code -12. Then, at > >>> > >>> d79025c7f5e3 drm/ttm: always initialize the full ttm_resource v2 > >>> > >>> the error code changes to the above -28. > >>> > >>> If I checkout one commit prior to d79025c7f5e3 and revert > >>> d02117f8efaa, things work again. There are a bunch of other TTM > >>> commits between this and HEAD, so reverting these on top of HEAD > >>> doesn't work. However, I checked that both yesterday's and today's > >>> nexts are also broken. > >>> > >>> Thank you, > >>> Mikko > >>> > >> ___ > >> Nouveau mailing list > >> nouv...@lists.freedesktop.org > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fnouveaudata=04%7C01%7Cchristian.koenig%40amd.com%7Caaf09cbea0b04d8dc01208d92b5637ba%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637588472445308290%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=ePoWVtHPXeK5RThkRuQSykKrfWCgPOzG5CLTzfw9%2Fuw%3Dreserved=0 >
Re: [Nouveau] Trouble with TTM patches w/nouveau in linux-next
Christian - potentially relevant is that Tegra doesn't have VRAM at all -- all GTT (or GART or whatever it's called nowadays). No fake/stolen VRAM. Cheers, -ilia On Wed, Jun 9, 2021 at 10:18 AM Christian König wrote: > > Hi Mikko, > > strange sounds like Nouveau was somehow also using the GEM workaround > for VMWGFX as well. > > But -12 means -ENOMEM which doesn't fits into the picture. > > I will try with a G710, but if that doesn't yields anything I need some > more input from you. > > Thanks for the report, > Christian. > > > Am 09.06.21 um 15:47 schrieb Mikko Perttunen: > > Hi, > > > > I'm observing nouveau not initializing recently on linux-next on my > > Tegra186 Jetson TX2 board. Specifically it looks like BO allocation is > > failing when initializing the sync subsystem: > > > > [ 21.858149] nouveau 1700.gpu: DRM: failed to initialise sync > > subsystem, -28 > > > > I have been bisecting and I have found two patches that affect this. > > Firstly, things first break on > > > > d02117f8efaa drm/ttm: remove special handling for non GEM drivers > > > > starting to return error code -12. Then, at > > > > d79025c7f5e3 drm/ttm: always initialize the full ttm_resource v2 > > > > the error code changes to the above -28. > > > > If I checkout one commit prior to d79025c7f5e3 and revert > > d02117f8efaa, things work again. There are a bunch of other TTM > > commits between this and HEAD, so reverting these on top of HEAD > > doesn't work. However, I checked that both yesterday's and today's > > nexts are also broken. > > > > Thank you, > > Mikko > > > > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
Re: [Nouveau] nouveau broken on Riva TNT2 in 5.13.0-rc4: NULL pointer dereference in nouveau_bo_sync_for_device
Another instance of a report like this here: https://gitlab.freedesktop.org/drm/nouveau/-/issues/92 On Sat, Jun 5, 2021 at 3:53 PM Ondrej Zary wrote: > > Hello, > I'm testing 5.13.0-rc4 and nouveau crashes with NULL pointer dereference in > nouveau_bo_sync_for_device. > Found various reports like this but that was back in februaryso that should > be fixed now. > > [ 21.003216] BUG: kernel NULL pointer dereference, address: > [ 21.003235] #PF: supervisor read access in kernel mode > [ 21.003243] #PF: error_code(0x) - not-present page > [ 21.003250] *pde = > [ 21.003258] Oops: [#1] SMP > [ 21.003268] CPU: 0 PID: 222 Comm: systemd-udevd Not tainted 5.13.0-rc4+ > #327 > [ 21.003278] Hardware name: /848P-ICH5, BIOS 6.00 PG 02/03/2005 > [ 21.003285] EIP: nouveau_bo_sync_for_device+0x9e/0xbf [nouveau] > [ 21.003571] Code: 02 89 45 e8 01 d1 8b 19 89 5d ec bb 01 00 00 00 3b 5d e8 > 74 0d 89 d8 c1 e0 05 03 45 ec 39 04 99 74 1e 8b 46 10 89 d9 c1 e1 0c <8b> 14 > 10 8b 47 e0 8b 40 08 6a 01 e8 d5 03 55 df 01 5d f0 58 eb ae > [ 21.003588] EAX: EBX: 0010 ECX: 0001 EDX: > [ 21.003597] ESI: c3e90280 EDI: c185a494 EBP: c2ed7c10 ESP: c2ed7bf8 > [ 21.003606] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00210206 > [ 21.003615] CR0: 80050033 CR2: CR3: 02ecb000 CR4: 0690 > [ 21.003625] Call Trace: > [ 21.003635] nouveau_bo_validate+0x3f/0x48 [nouveau] > [ 21.003911] nouveau_bo_pin+0xf0/0x187 [nouveau] > [ 21.004182] nouveau_channel_prep+0xc0/0x269 [nouveau] > [ 21.004454] nouveau_channel_new+0x3c/0x5f5 [nouveau] > [ 21.004725] ? slab_free_freelist_hook+0x3b/0xa7 > [ 21.004740] ? kfree+0x9e/0x11a > [ 21.004749] ? nvif_object_sclass_put+0xd/0x16 [nouveau] > [ 21.004944] nouveau_drm_device_init+0x2e2/0x646 [nouveau] > [ 21.005186] ? pci_enable_device_flags+0x23/0x97 > [ 21.005202] nouveau_drm_probe+0xe5/0x182 [nouveau] > [ 21.005443] ? nouveau_drm_device_init+0x646/0x646 [nouveau] > [ 21.005683] pci_device_probe+0x89/0xe9 > [ 21.005696] really_probe+0x127/0x2b9 > [ 21.005707] driver_probe_device+0x62/0x89 > [ 21.005715] device_driver_attach+0x2e/0x41 > [ 21.005724] __driver_attach+0x83/0x8a > [ 21.005732] bus_for_each_dev+0x4c/0x66 > [ 21.005740] driver_attach+0x14/0x16 > [ 21.005747] ? device_driver_attach+0x41/0x41 > [ 21.005756] bus_add_driver+0xc5/0x16c > [ 21.005764] driver_register+0x87/0xb9 > [ 21.005772] __pci_register_driver+0x38/0x3b > [ 21.005780] ? 0xf0be4000 > [ 21.005787] nouveau_drm_init+0x14c/0x1000 [nouveau] > [ 21.005964] do_one_initcall+0x5a/0x134 > [ 21.005975] ? __vunmap+0x124/0x12d > [ 21.005984] ? __vunmap+0x124/0x12d > [ 21.005992] ? kmem_cache_alloc+0xa8/0xb6 > [ 21.006001] ? do_init_module+0x17/0x1cf > [ 21.006012] do_init_module+0x46/0x1cf > [ 21.006021] load_module+0x1799/0x1bcb > [ 21.006032] __ia32_sys_finit_module+0x72/0x7a > [ 21.006044] do_int80_syscall_32+0x53/0x62 > [ 21.006054] entry_INT80_32+0xf0/0xf0 > [ 21.006063] EIP: 0xb7f40092 > [ 21.006071] Code: 00 00 00 e9 90 ff ff ff ff a3 24 00 00 00 68 30 00 00 00 > e9 80 ff ff ff ff a3 e8 ff ff ff 66 90 00 00 00 00 00 00 00 00 cd 80 8d > b4 26 00 00 00 00 8d b6 00 00 00 00 8b 1c 24 c3 8d b4 26 00 > [ 21.006086] EAX: ffda EBX: 0010 ECX: b7e9bbdd EDX: > [ 21.006095] ESI: 008f27d0 EDI: 008f9e10 EBP: ESP: bfa140b8 > [ 21.006103] DS: 007b ES: 007b FS: GS: 0033 SS: 007b EFLAGS: 00200296 > [ 21.006114] Modules linked in: nouveau(+) snd_intel8x0 snd_ac97_codec > pcmcia wmi hwmon ac97_bus yenta_socket pcmcia_rsrc drm_ttm_helper snd_pcm ttm > snd_timer pcmcia_core psmouse 8139cp snd sg soundcore serio_raw parport_pc > intel_agp parport > [ 21.006165] CR2: > [ 21.006201] ---[ end trace 02dc541683feafc6 ]--- > [ 21.006211] EIP: nouveau_bo_sync_for_device+0x9e/0xbf [nouveau] > [ 21.006460] Code: 02 89 45 e8 01 d1 8b 19 89 5d ec bb 01 00 00 00 3b 5d e8 > 74 0d 89 d8 c1 e0 05 03 45 ec 39 04 99 74 1e 8b 46 10 89 d9 c1 e1 0c <8b> 14 > 10 8b 47 e0 8b 40 08 6a 01 e8 d5 03 55 df 01 5d f0 58 eb ae > [ 21.006476] EAX: EBX: 0010 ECX: 0001 EDX: > [ 21.006485] ESI: c3e90280 EDI: c185a494 EBP: c2ed7c10 ESP: c2ed7bf8 > [ 21.006494] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068 EFLAGS: 00210206 > [ 21.006503] CR0: 80050033 CR2: CR3: 02ecb000 CR4: 0690 > > > -- > Ondrej Zary > ___ > Nouveau mailing list > nouv...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/nouveau
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(_connector->aux);
> - drm_dp_aux_unregister(_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(_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(_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(_connector->aux);
> + drm_dp_aux_init(_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 = _connector_funcs;
> - break;
> + fallthrough;
> default:
> funcs = _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: [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: [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
[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(>flip); nouveau_display(dev)->priv = NULL; - kfree(disp); + vfree(disp); nvif_object_unmap(>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: [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
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: [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 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
[+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, unsigned int > int ret; > > /* maybe make cursor width/height configurable some day */ > - uint32_t cw = 64; > -
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
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(>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 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(>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: [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 = >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(_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(>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 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, ); > - 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
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] 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 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: [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 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 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] 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] drm/edid: Fix uninitialized variable in drm_cvt_modes()
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)?
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: It appears drm-next TTM cleanup broke something . . .
On Tue, Oct 20, 2020 at 1:17 PM Alex Deucher wrote: > > On Mon, Oct 19, 2020 at 3:43 PM Kevin Brace wrote: > > > > Hi Sam, > > > > Thanks for asking the question. > > The current OpenChrome DRM code has these two major issues. > > > > 1) It does not support atomic modesetting > > > > I do internally have working code to support atomic modesetting, but it is > > not ready for committing into the upstream OpenChrome DRM repository. > > In particular, it suffers from a freeze relating to a cursor plane. > > The freeze is a bad kind that kern.log does not really tell me what is > > wrong. > > If I disable hardware cursor, the atomic modesetting based OpenChrome DRM > > appears to work okay. > > In other words, I am getting close to getting atomic modesetting working, > > but I am stuck. > > > > > > 2) Double allocation of visible portion of frame buffer > > > > This is a big problem left behind from the previous developer who developed > > OpenChrome prior to me. > > For some reason, the developer wanted to allocate visible portion of the > > frame buffer to be the maximum possible size supported by the detected > > monitor when initializing the frame buffer inside OpenChrome DRM code. > > I believe Radeon DRM does something similar to that. > > The problem is, OpenChrome DDX allocates an equal sized frame buffer > > visible portion during the DDX's initialization. > > This means that we got two same sized visible portions allocated, but > > OpenChrome DDX and OpenChrome DRM combined should really be allocating only > > one. > > At this point, OpenChrome is not supporting double buffering. > > This double allocation of a visible portion of the frame buffer contributes > > to a X Server crash when the screen is resized and 16 MB or less (i.e., 8 > > MB) shared frame buffer is reserved by the system via BIOS setup. > > I personally think letting OpenChrome DRM allocate the visible portion of > > the frame buffer is the way to go, but if so, how do I get the DDX or > > shadow FB to access the frame buffer visible portion allocated by > > OpenChrome DRM? > > Any suggestions on what to do about this issue will be greatly appreciated. > > All drivers do this. You have one buffer for the fbdev console and > then userspace allocates buffers it needs in addition, otherwise, > you'd overwrite your console. You can disable it by not enabling the > fbdev emulation if you don't want a console. I don't think disabling fbdev emulation is a practical thing to do for standard PCs. Not sure what the capabilities of the underlying hardware are, but on nouveau, we use 16bpp/8bpp framebuffers by default (for fbdev) on boards with limited VRAM (64/32/16 MB, I don't remember precisely). You'd have to add C8 drm format support for this, but assuming you have that, the core supports it OK. Ideally you'd have logic which is able to manage buffers and move them between GPU-accessible and GPU-inaccessible memory (e.g. TTM, or perhaps some evolution of it, I'm not up on all the latest). Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] nouveau broken on Riva TNT2 in 5.9.0-rc8: GPU not supported on big-endian
On Fri, Oct 9, 2020 at 5:54 PM Karol Herbst wrote: > > On Fri, Oct 9, 2020 at 11:35 PM Ondrej Zary wrote: > > > > Hello, > > I'm testing 5.9.0-rc8 and found that Riva TNT2 stopped working: > > [0.00] Linux version 5.9.0-rc8+ (zary@gsql) (gcc (Debian 8.3.0-6) > > 8.3.0, GNU ld (GNU Binutils for Debian) 2.31.1) #326 SMP Fri Oct 9 22:31:40 > > CEST 2020 > > ... > > [ 14.771464] nouveau :01:00.0: GPU not supported on big-endian > > [ 14.771782] nouveau: probe of :01:00.0 failed with error -38 > > > > big-endian? WTF? The machine is x86. > > > > mhh, we reworked the endianess checks a bit and apparently that broke > something... I will give it some thoughts, but could you be so kind > and create an mmiotrace under 5.9 with nouveau? You won't need to > start X or anything while doing it. Just enable the trace and modprobe > nouveau and collect the trace. Looks like nvkm_device_endianness unconditionally reads out 0x4. I don't think that reg is there pre-NV11. At least NV4, NV5, NV10 and maybe NV15 (which is logically pre-NV11) don't support big-endian mode. Not sure about NV1A, which was the IGP of the series and IIRC logically pre-NV11 as well (but clearly could only be used with x86 chips, since it was part of the motherboard). Aha, it's documented in rnndb: https://github.com/envytools/envytools/blob/master/rnndb/bus/pmc.xml -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
On Fri, Sep 25, 2020 at 6:08 PM Lyude Paul wrote: > > On Tue, 2020-09-22 at 17:22 -0400, Ilia Mirkin wrote: > > On Tue, Sep 22, 2020 at 5:14 PM Lyude Paul wrote: > > > On Tue, 2020-09-22 at 17:10 -0400, Ilia Mirkin wrote: > > > > Can we use 6bpc on arbitrary DP monitors, or is there a capability for > > > > it? Maybe only use 6bpc if display_info.bpc == 6 and otherwise use 8? > > > > > > I don't think that display_info.bpc actually implies a minimum bpc, only a > > > maximum bpc iirc (Ville would know the answer to this one). The other > > > thing > > > to > > > note here is that we want to assume the lowest possible bpc here since > > > we're > > > only concerned if the mode passed to ->mode_valid can be set under -any- > > > conditions (including those that require lowering the bpc beyond it's > > > maximum > > > value), so we definitely do want to always use 6bpc here even once we get > > > support for optimizing the bpc based on the available display bandwidth. > > > > Yeah, display_info is the max bpc. But would an average monitor > > support 6bpc? And if it does, does the current link training code even > > try that when display_info.bpc != 6? > > So I did confirm that 6bpc support is mandatory for DP, so yes-6 bpc will > always > work. > > But also, your second comment doesn't really apply here. So: to be clear, > we're > not really concerned here about whether nouveau will actually use 6bpc or not. > In truth I'm not actually sure either if we have any code that uses 6bpc (iirc > we don't), since we don't current optimize bpc. I think it's very possible for > us to use 6bpc for eDP displays if I recall though, but I'm not sure on that. > > But that's also not the point of this code. ->mode_valid() is only used in two > situations in DRM modesetting: when probing connector modes, and when checking > if a mode is valid or not during the atomic check for atomic modesetting. Its > purpose is only to reject display modes that are physically impossible to set > in > hardware due to static hardware constraints. Put another way, we only check > the > given mode against constraints which will always remain constant regardless of > the rest of the display state. An example of a static constraint would be the > max pixel clock supported by the hardware, since on sensible hardware this > never > changes. A dynamic constraint would be something like how much bandwidth is > currently unused on an MST topology, since that value is entirely dependent on > the rest of the display state. > > So - with that said, bpc is technically a dynamic constraint because while a > sink and source both likely have their own bpc limits, any bpc which is equal > or > below that limit can be used depending on what the driver decides - which will > be based on the max_bpc property, and additionally for MST displays it will > also > depend on the available bandwidth on the topology. The only non-dynamic thing > about bpc is that at a minimum, it will be 6 - so any mode that doesn't fit on > the link with a bpc of 6 is guaranteed to be a mode that we'll never be able > to > set and therefore want to prune. > > So, even if we're not using 6 in the majority of situations, I'm fairly > confident it's the right value here. It's also what i915 does as well (and > they > previously had to fix a bug that was the result of assuming a minimum of 8bpc > instead of 6). Here's the situation I'm trying to avoid: 1. Mode is considered "OK" from a bandwidth perspective @6bpc 2. Modesetting logic never tries 6bpc and the modeset fails As long as the two bits of logic agree on what is settable, I'm happy. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
On Tue, Sep 22, 2020 at 5:14 PM Lyude Paul wrote: > > On Tue, 2020-09-22 at 17:10 -0400, Ilia Mirkin wrote: > > Can we use 6bpc on arbitrary DP monitors, or is there a capability for > > it? Maybe only use 6bpc if display_info.bpc == 6 and otherwise use 8? > > I don't think that display_info.bpc actually implies a minimum bpc, only a > maximum bpc iirc (Ville would know the answer to this one). The other thing to > note here is that we want to assume the lowest possible bpc here since we're > only concerned if the mode passed to ->mode_valid can be set under -any- > conditions (including those that require lowering the bpc beyond it's maximum > value), so we definitely do want to always use 6bpc here even once we get > support for optimizing the bpc based on the available display bandwidth. Yeah, display_info is the max bpc. But would an average monitor support 6bpc? And if it does, does the current link training code even try that when display_info.bpc != 6? -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid()
Can we use 6bpc on arbitrary DP monitors, or is there a capability for
it? Maybe only use 6bpc if display_info.bpc == 6 and otherwise use 8?
On Tue, Sep 22, 2020 at 5:06 PM Lyude Paul wrote:
>
> While I thought I had this correct (since it actually did reject modes
> like I expected during testing), Ville Syrjala from Intel pointed out
> that the logic here isn't correct. max_clock refers to the max symbol
> rate supported by the encoder, so limiting clock to ds_clock using max()
> doesn't make sense. Additionally, we want to check against 6bpc for the
> time being since that's the minimum possible bpc here, not the reported
> bpc from the connector. See:
>
> https://lists.freedesktop.org/archives/dri-devel/2020-September/280276.html
>
> For more info.
>
> So, let's rewrite this using Ville's advice.
>
> Signed-off-by: Lyude Paul
> Fixes: 409d38139b42 ("drm/nouveau/kms/nv50-: Use downstream DP clock limits
> for mode validation")
> Cc: Ville Syrjälä
> Cc: Lyude Paul
> Cc: Ben Skeggs
> ---
> drivers/gpu/drm/nouveau/nouveau_dp.c | 23 +--
> 1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_dp.c
> b/drivers/gpu/drm/nouveau/nouveau_dp.c
> index 7b640e05bd4cd..24c81e423d349 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_dp.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_dp.c
> @@ -231,23 +231,26 @@ nv50_dp_mode_valid(struct drm_connector *connector,
>const struct drm_display_mode *mode,
>unsigned *out_clock)
> {
> - const unsigned min_clock = 25000;
> - unsigned max_clock, ds_clock, clock;
> + const unsigned int min_clock = 25000;
> + unsigned int max_clock, ds_clock, clock;
> + const u8 bpp = 18; /* 6 bpc */
> enum drm_mode_status ret;
>
> if (mode->flags & DRM_MODE_FLAG_INTERLACE && !outp->caps.dp_interlace)
> return MODE_NO_INTERLACE;
>
> max_clock = outp->dp.link_nr * outp->dp.link_bw;
> - ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
> - outp->dp.downstream_ports);
> - if (ds_clock)
> - max_clock = min(max_clock, ds_clock);
> -
> - clock = mode->clock * (connector->display_info.bpc * 3) / 10;
> - ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
> - );
> + clock = mode->clock * bpp / 8;
> + if (clock > max_clock)
> + return MODE_CLOCK_HIGH;
> +
> + ds_clock = drm_dp_downstream_max_dotclock(outp->dp.dpcd,
> outp->dp.downstream_ports);
> + if (ds_clock && mode->clock > ds_clock)
> + return MODE_CLOCK_HIGH;
> +
> + ret = nouveau_conn_mode_clock_valid(mode, min_clock, max_clock,
> );
> if (out_clock)
> *out_clock = clock;
> +
> return ret;
> }
> --
> 2.26.2
>
> ___
> 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: [git pull] drm for 5.8-rc1
On Wed, Aug 12, 2020 at 8:24 AM Karol Herbst wrote:
>
> On Wed, Aug 12, 2020 at 12:43 PM Karol Herbst wrote:
> >
> > On Wed, Aug 12, 2020 at 12:27 PM Karol Herbst wrote:
> > >
> > > On Wed, Aug 12, 2020 at 2:19 AM James Jones wrote:
> > > >
> > > > Sorry for the slow reply here as well. I've been in the process of
> > > > rebasing and reworking the userspace patches. I'm not clear my changes
> > > > will address the Jetson Nano issue, but if you'd like to try them, the
> > > > latest userspace changes are available here:
> > > >
> > > >https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/3724
> > > >
> > > > And the tegra-drm kernel patches are here:
> > > >
> > > >
> > > > https://patchwork.ozlabs.org/project/linux-tegra/patch/20191217005205.2573-1-jajo...@nvidia.com/
> > > >
> > > > Those + the kernel changes addressed in this thread are everything I had
> > > > outstanding.
> > > >
> > >
> > > I don't know if that's caused by your changes or not, but now the
> > > assert I hit is a different one pointing out that
> > > nvc0_miptree_select_best_modifier fails in a certain case and returns
> > > MOD_INVALID... anyway, it seems like with your patches applied it's
> > > now way easier to debug and figure out what's going wrong, so maybe I
> > > can figure it out now :)
> > >
> >
> > collected some information which might help to track it down.
> >
> > src/gallium/frontends/dri/dri2.c:648 is the assert hit: assert(*zsbuf)
> >
> > templ is {reference = {count = 0}, width0 = 300, height0 = 300, depth0
> > = 1, array_size = 1, format = PIPE_FORMAT_Z24X8_UNORM, target =
> > PIPE_TEXTURE_2D, last_level = 0, nr_samples = 0, nr_storage_samples =
> > 0, usage = 0, bind = 1, flags = 0, next = 0x0, screen = 0x0}
> >
> > inside tegra_screen_resource_create modifier says
> > DRM_FORMAT_MOD_INVALID as template->bind is 1
> >
> > and nvc0_miptree_select_best_modifier returns DRM_FORMAT_MOD_INVALID,
> > so the call just returns NULL leading to the assert.
> >
> > Btw, this is on Xorg-1.20.8-1.fc32.aarch64 with glxgears.
> >
>
> So I digged a bit deeper and here is what tripps it of:
>
> when the context gets made current, the normal framebuffer validation
> and render buffer allocation is done, but we end up inside
> tegra_screen_resource_create at some point with PIPE_BIND_SCANOUT set
> in template->bind. Now the tegra driver forces the
> DRM_FORMAT_MOD_LINEAR modifier and calls into
> resource_create_with_modifiers.
>
> If it wouldn't do that, nouveau would allocate a tiled buffer, with
> that it's linear and we at some point end up with an assert about a
> depth_stencil buffer being there even though it shouldn't. If I always
> use DRM_FORMAT_MOD_INVALID in tegra_screen_resource_create, things
> just work.
>
> That's kind of the cause I pinpointed the issue down to. But I have no
> idea what's supposed to happen and what the actual bug is.
Yeah, the bug with tegra has always been "trying to render to linear
color + tiled depth", which the hardware plain doesn't support. (And
linear depth isn't a thing.)
Question is whether what it's doing necessary. PIPE_BIND_SCANOUT
(/linear) requirements are needed for DRI2 to work (well, maybe not in
theory, but at least in practice the nouveau ddx expects linear
buffers). However tegra operates on a more DRI3-like basis, so with
"client" allocations, tiled should work OK as long as there's
something in tegra to copy it to linear when necessary?
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] 2dd4d163cd9c ("drm/nouveau: remove open-coded version of remove_conflicting_pci_framebuffers()")
Hi Boris, There was a fixup to that patch that you'll also have to revert first -- 7dbbdd37f2ae7dd4175ba3f86f4335c463b18403. I guess there's some subtle difference between the old open-coded logic and the helper, they were supposed to be identical. Cheers, -ilia On Thu, Jun 18, 2020 at 4:09 PM Borislav Petkov wrote: > > Hi, > > my test box won't boot 5.8-rc1 all the way but stops at > > ... > fb0: switching to nouveaufb from EFI VGA > <-- EOF > > I've bisected it to the commit in $Subject, see below. Unfortunately, it > doesn't revert cleanly so I can't really do the final test of reverting > it ontop of 5.8-rc1 to confirm that this one is really causing it. > > Any ideas? > > GPU is: > > [5.678614] fb0: switching to nouveaufb from EFI VGA > [5.685577] Console: switching to colour dummy device 80x25 > [5.691865] nouveau :03:00.0: NVIDIA GT218 (0a8c00b1) > [5.814409] nouveau :03:00.0: bios: version 70.18.83.00.08 > [5.823559] nouveau :03:00.0: fb: 512 MiB DDR3 > [6.096680] [TTM] Zone kernel: Available graphics memory: 8158364 KiB > [6.103327] [TTM] Zone dma32: Available graphics memory: 2097152 KiB > [6.109951] [TTM] Initializing pool allocator > [6.114405] [TTM] Initializing DMA pool allocator > [6.119256] nouveau :03:00.0: DRM: VRAM: 512 MiB > [6.124285] nouveau :03:00.0: DRM: GART: 1048576 MiB > [6.129677] nouveau :03:00.0: DRM: TMDS table version 2.0 > [6.135534] nouveau :03:00.0: DRM: DCB version 4.0 > [6.140755] nouveau :03:00.0: DRM: DCB outp 00: 02000360 > [6.147273] nouveau :03:00.0: DRM: DCB outp 01: 02000362 00020010 > [6.153782] nouveau :03:00.0: DRM: DCB outp 02: 028003a6 0f220010 > [6.160292] nouveau :03:00.0: DRM: DCB outp 03: 01011380 > [6.166810] nouveau :03:00.0: DRM: DCB outp 04: 08011382 00020010 > [6.173306] nouveau :03:00.0: DRM: DCB outp 05: 088113c6 0f220010 > [6.179829] nouveau :03:00.0: DRM: DCB conn 00: 00101064 > [6.185553] nouveau :03:00.0: DRM: DCB conn 01: 00202165 > [6.196145] nouveau :03:00.0: DRM: MM: using COPY for buffer copies > [6.233659] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [6.311939] nouveau :03:00.0: DRM: allocated 1920x1080 fb: 0x7, bo > (ptrval) > [6.320736] fbcon: nouveaudrmfb (fb0) is primary device > [6.392722] tsc: Refined TSC clocksource calibration: 3591.346 MHz > [6.392788] clocksource: tsc: mask: 0x max_cycles: > 0x33c46403b59, max_idle_ns: 440795293818 ns > [6.392930] clocksource: Switched to clocksource tsc > [6.509946] Console: switching to colour frame buffer device 240x67 > [6.546287] nouveau :03:00.0: fb0: nouveaudrmfb frame buffer device > [6.555021] [drm] Initialized nouveau 1.3.1 20120801 for :03:00.0 on > minor 0 > > Thx. > > git bisect start > # good: [3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162] Linux 5.7 > git bisect good 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162 > # bad: [b3a9e3b9622ae10064826dccb4f7a52bd88c7407] Linux 5.8-rc1 > git bisect bad b3a9e3b9622ae10064826dccb4f7a52bd88c7407 > # bad: [ee01c4d72adffb7d424535adf630f2955748fa8b] Merge branch 'akpm' > (patches from Andrew) > git bisect bad ee01c4d72adffb7d424535adf630f2955748fa8b > # bad: [16d91548d1057691979de4686693f0ff92f46000] Merge tag 'xfs-5.8-merge-8' > of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux > git bisect bad 16d91548d1057691979de4686693f0ff92f46000 > # good: [cfa3b8068b09f25037146bfd5eed041b78878bee] Merge tag 'for-linus-hmm' > of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma > git bisect good cfa3b8068b09f25037146bfd5eed041b78878bee > # good: [3fd911b69b3117e03181262fc19ae6c3ef6962ce] Merge tag > 'drm-misc-next-2020-05-07' of git://anongit.freedesktop.org/drm/drm-misc into > drm-next > git bisect good 3fd911b69b3117e03181262fc19ae6c3ef6962ce > # bad: [1966391fa576e1fb2701be8bcca197d8f72737b7] mm/migrate.c: > attach_page_private already does the get_page > git bisect bad 1966391fa576e1fb2701be8bcca197d8f72737b7 > # good: [43c8546bcd854806736d8a635a0d696504dd4c21] drm/amdgpu: Add a UAPI > flag for user to call mem_sync > git bisect good 43c8546bcd854806736d8a635a0d696504dd4c21 > # good: [6cf991611bc72c077f0cc64e23987341ad7ef41e] Merge tag > 'drm-intel-next-2020-05-15' of git://anongit.freedesktop.org/drm/drm-intel > into drm-next > git bisect good 6cf991611bc72c077f0cc64e23987341ad7ef41e > # bad: [dc455f4c888365595c0a13da445e092422d55b8d] drm/nouveau/dispnv50: fix > runtime pm imbalance on error > git bisect bad dc455f4c888365595c0a13da445e092422d55b8d > # bad: [2dd4d163cd9c15432524aa9863155bc03a821361] drm/nouveau: remove > open-coded version of remove_conflicting_pci_framebuffers() > git bisect bad 2dd4d163cd9c15432524aa9863155bc03a821361 > # good: [c41219fda6e04255c44d37fd2c0d898c1c46abf1] Merge tag > 'drm-intel-next-fixes-2020-05-20' of >
Re: [Nouveau] [PATCH] nouveau: add fbdev dependency
Isn't this already fixed by
https://cgit.freedesktop.org/drm/drm/commit/?id=7dbbdd37f2ae7dd4175ba3f86f4335c463b18403
On Wed, May 27, 2020 at 9:43 AM Arnd Bergmann wrote:
>
> Calling directly into the fbdev stack only works when the
> fbdev layer is built into the kernel as well, or both are
> loadable modules:
>
> drivers/gpu/drm/nouveau/nouveau_drm.o: in function `nouveau_drm_probe':
> nouveau_drm.c:(.text+0x1f90): undefined reference to
> `remove_conflicting_pci_framebuffers'
>
> The change seems to have been intentional, so add an explicit
> dependency here but allow it to still be compiled if FBDEV
> is completely disabled.
>
> Fixes: 2dd4d163cd9c ("drm/nouveau: remove open-coded version of
> remove_conflicting_pci_framebuffers()")
> Signed-off-by: Arnd Bergmann
> ---
> drivers/gpu/drm/nouveau/Kconfig | 1 +
> drivers/gpu/drm/nouveau/nouveau_drm.c | 3 ++-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
> index 980ed09bd7f6..8c640f003358 100644
> --- a/drivers/gpu/drm/nouveau/Kconfig
> +++ b/drivers/gpu/drm/nouveau/Kconfig
> @@ -18,6 +18,7 @@ config DRM_NOUVEAU
> select THERMAL if ACPI && X86
> select ACPI_VIDEO if ACPI && X86
> select SND_HDA_COMPONENT if SND_HDA_CORE
> + depends on FBDEV || !FBDEV
> help
> Choose this option for open-source NVIDIA support.
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c
> b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index eb10c80ed853..e8560444ab57 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -697,7 +697,8 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
> nvkm_device_del();
>
> /* Remove conflicting drivers (vesafb, efifb etc). */
> - ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
> + if (IS_ENABLED(CONFIG_FBDEV))
> + ret = remove_conflicting_pci_framebuffers(pdev, "nouveaufb");
> if (ret)
> return ret;
>
> --
> 2.26.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 v3 3/5] drm/nouveau/kms/gv100-: Add support for interlaced modes
On Mon, May 11, 2020 at 6:42 PM Lyude Paul wrote: > diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c > b/drivers/gpu/drm/nouveau/nouveau_connector.c > index 43bcbb6d73c4..6dae00da5d7e 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_connector.c > +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c > @@ -1065,7 +1065,7 @@ nouveau_connector_mode_valid(struct drm_connector > *connector, > return get_slave_funcs(encoder)->mode_valid(encoder, mode); > case DCB_OUTPUT_DP: > if (mode->flags & DRM_MODE_FLAG_INTERLACE && > - !nv_encoder->dp.caps.interlace) > + !nv_encoder->caps.dp_interlace) > return MODE_NO_INTERLACE; > > max_clock = nv_encoder->dp.link_nr; You probably meant for this hunk to go into an earlier patch. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau/dispnv04: Remove dead code
On Fri, May 1, 2020 at 9:15 AM Souptick Joarder wrote: > > On Fri, May 1, 2020 at 2:21 AM Ilia Mirkin wrote: > > > > Interesting. I do remember seeing some snow on NV5's overlay at high > > resolutions. Wonder if it was because of this missing code... > > What was the problem ? Does enabling this dead code will fix the problem ? This "dead" code is essentially documentation. It's from the xf86-video-nv driver which was the base of the dispnv04 code. It won't compile as-is, which is why it's #if 0'd out. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [PATCH] drm/nouveau/dispnv04: Remove dead code
Interesting. I do remember seeing some snow on NV5's overlay at high
resolutions. Wonder if it was because of this missing code...
On Thu, Apr 30, 2020 at 4:19 PM Souptick Joarder wrote:
>
> These are dead code since 3.10. If there is no plan to use
> it further, these can be removed forever.
>
> Signed-off-by: Souptick Joarder
> ---
> drivers/gpu/drm/nouveau/dispnv04/crtc.c | 28
> 1 file changed, 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 1f08de4..ad0ef7d 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -269,31 +269,11 @@ static void nv_crtc_calc_state_ext(struct drm_crtc
> *crtc, struct drm_display_mod
> horizStart = horizTotal - 5;
> horizEnd = horizTotal - 2;
> horizBlankEnd = horizTotal + 4;
> -#if 0
> - if (dev->overlayAdaptor && drm->client.device.info.family >=
> NV_DEVICE_INFO_V0_CELSIUS)
> - /* This reportedly works around some video overlay
> bandwidth problems */
> - horizTotal += 2;
> -#endif
> }
>
> if (mode->flags & DRM_MODE_FLAG_INTERLACE)
> vertTotal |= 1;
>
> -#if 0
> - ErrorF("horizDisplay: 0x%X \n", horizDisplay);
> - ErrorF("horizStart: 0x%X \n", horizStart);
> - ErrorF("horizEnd: 0x%X \n", horizEnd);
> - ErrorF("horizTotal: 0x%X \n", horizTotal);
> - ErrorF("horizBlankStart: 0x%X \n", horizBlankStart);
> - ErrorF("horizBlankEnd: 0x%X \n", horizBlankEnd);
> - ErrorF("vertDisplay: 0x%X \n", vertDisplay);
> - ErrorF("vertStart: 0x%X \n", vertStart);
> - ErrorF("vertEnd: 0x%X \n", vertEnd);
> - ErrorF("vertTotal: 0x%X \n", vertTotal);
> - ErrorF("vertBlankStart: 0x%X \n", vertBlankStart);
> - ErrorF("vertBlankEnd: 0x%X \n", vertBlankEnd);
> -#endif
> -
> /*
> * compute correct Hsync & Vsync polarity
> */
> @@ -492,14 +472,6 @@ static void nv_crtc_calc_state_ext(struct drm_crtc
> *crtc, struct drm_display_mod
> /* Except for rare conditions I2C is enabled on the primary crtc */
> if (nv_crtc->index == 0)
> regp->crtc_eng_ctrl |= NV_CRTC_FSEL_I2C;
> -#if 0
> - /* Set overlay to desired crtc. */
> - if (dev->overlayAdaptor) {
> - NVPortPrivPtr pPriv = GET_OVERLAY_PRIVATE(dev);
> - if (pPriv->overlayCRTC == nv_crtc->index)
> - regp->crtc_eng_ctrl |= NV_CRTC_FSEL_OVERLAY;
> - }
> -#endif
>
> /* ADDRESS_SPACE_PNVM is the same as setting HCUR_ASI */
> regp->cursor_cfg = NV_PCRTC_CURSOR_CONFIG_CUR_LINES_64 |
> --
> 1.9.1
>
> ___
> 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 libdrm] modetest: set_gamma only if CRTC supports gamma property
Not an enormous fan of what you had to do in atomic_set_planes, but
OTOH I don't see a much better way to do it either.
Reviewed-by: Ilia Mirkin
On Tue, Mar 17, 2020 at 8:11 AM Rohit Visavalia
wrote:
>
> Current implementation shows error as "failed to set gamma: Function
> no implemented" if platform specific drm has no gamma property implemented
>
> Signed-off-by: Rohit Visavalia
> ---
> tests/modetest/modetest.c | 21 -
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/tests/modetest/modetest.c b/tests/modetest/modetest.c
> index b907ab3..23ee73d 100644
> --- a/tests/modetest/modetest.c
> +++ b/tests/modetest/modetest.c
> @@ -1328,14 +1328,24 @@ 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 = primary_fill;
> + unsigned int i, j, pattern = primary_fill;
> + struct crtc *crtc = NULL;
>
> /* set up planes */
> for (i = 0; i < count; i++) {
> - if (i > 0)
> + if (i > 0) {
> pattern = secondary_fill;
> - else
> - set_gamma(dev, p[i].crtc_id, p[i].fourcc);
> + } else {
> + for (j = 0; j < dev->resources->res->count_crtcs;
> j++) {
> + if (p[i].crtc_id ==
> + dev->resources->res->crtcs[j]) {
> + crtc = >resources->crtcs[j];
> + break;
> + }
> + }
> + if (crtc->crtc->gamma_size)
> + set_gamma(dev, p[i].crtc_id, p[i].fourcc);
> + }
>
> if (atomic_set_plane(dev, [i], pattern, update))
> return;
> @@ -1522,7 +1532,8 @@ static void set_mode(struct device *dev, struct
> pipe_arg *pipes, unsigned int co
> return;
> }
>
> - set_gamma(dev, pipe->crtc->crtc->crtc_id, pipe->fourcc);
> + if (pipe->crtc->crtc->gamma_size)
> + set_gamma(dev, pipe->crtc->crtc->crtc_id,
> pipe->fourcc);
> }
> }
>
> --
> 2.7.4
>
> This email and any attachments are intended for the sole use of the named
> recipient(s) and contain(s) confidential information that may be proprietary,
> privileged or copyrighted under applicable law. If you are not the intended
> recipient, do not read, copy, or forward this email message or any
> attachments. Delete this email message and any attachments immediately.
___
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 > > > > > Cc: Hyun Kwon ; Ranganathan Sk
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
[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
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: [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(>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 = _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 = _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 = _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 = _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_drv.c
> +++
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=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: [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: [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 @@
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: 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: [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 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: 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: [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: [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 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 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: [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 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],
> _burst, _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: [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 >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 >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 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 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
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(>mgr, mstc->pbn);
> r = drm_dp_mst_allocate_vcpi(>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 v4 3/7] drm: Add DisplayPort colorspace property
On Tue, Sep 10, 2019 at 9:21 AM Ilia Mirkin wrote: > > On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong > wrote: > > > > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote: > > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong > > > wrote: > > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > > > wrote: > > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +0000, Shankar, Uma wrote: > > > > > > > > -Original Message- > > > > > > > > From: Ilia Mirkin > > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > > > To: Mun, Gwan-gyeong > > > > > > > > Cc: Intel Graphics Development < > > > > > > > > intel-...@lists.freedesktop.org > > > > > > > > > ; Shankar, Uma > > > > > > > > ; dri-devel < > > > > > > > > dri-devel@lists.freedesktop.org> > > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > > > property > > > > > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > > > list > > > > > > > > the HDMI or DP > > > > > > > > properties? Or do we need a custom property checker which > > > > > > > > is > > > > > > > > aware of what is > > > > > > > > currently plugged in to validate the values? > > > > > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > > > DP > > > > > > > or HDMI (with a passive dongle). > > > > > > > Based on the type of sink detected, we should expose DP or > > > > > > > HDMI > > > > > > > colorspaces to userspace. > > > > > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > > > always > > > > > > drives > > > > > > HDMI mode, even when the physical connector is DP++. > > > > > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > > > amdgpu > > > > > create 1 connector (not sure about other drivers) for a single > > > > > physical DP++ socket. Since we supply the list of valid values at > > > > > the > > > > > time of creating the connector, we can't know at that point > > > > > whether > > > > > in > > > > > the future a HDMI or DP will be plugged into it. > > > > > > > > > > -ilia > > > > Ilia, does it mean that the drm_connector type is > > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > > > > > That is correct. The connector type is "DisplayPort" in such a case. > > > > > > Cheers, > > > > > > -ilia > > > > For now drm_mode_create_colorspace_property() is only used for i915. > > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for > > using of drm_mode_create_colorspace_property(), > > what about do we add a variable which can identify DP++ and DP to > > drm_connector? > > And when the drivers (nouveau, radeon, and amdgpu) detect the current > > protocol, the drivers will set the variable. > > I've been working on adding this to nouveau. > > Can/should such properties be added/removed at "runtime", rather than > at connector creation time? Either way, the function > drm_mode_create_colorspace_property as proposed would not be reusable, > since it looks at the connector type, which will always be > "DisplayPort" in such cases. Summary of conversation Ville and I had on IRC: - DP++ connectors to provide a single combined list of options for colorspace - set_property hook will check against currently plugged in thing, and reject incorrect values - in the case where someone sets e.g. an HDMI value for an HDMI-plugged-in thing, and then unplugs, and then plugs in a DP screen, the modeset should continue to succeed but use a default colorspace value. I think there was a bit of contention on that last point. Open to opinions, but we should try to avoid putting undue burden on esp non-atomic userspace. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong wrote: > > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote: > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong > > wrote: > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > > > wrote: > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > > -Original Message- > > > > > > > From: Ilia Mirkin > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > > > To: Mun, Gwan-gyeong > > > > > > > Cc: Intel Graphics Development < > > > > > > > intel-...@lists.freedesktop.org > > > > > > > > ; Shankar, Uma > > > > > > > ; dri-devel < > > > > > > > dri-devel@lists.freedesktop.org> > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > > > property > > > > > > > > > > > > > > So how would this work with a DP++ connector? Should it > > > > > > > list > > > > > > > the HDMI or DP > > > > > > > properties? Or do we need a custom property checker which > > > > > > > is > > > > > > > aware of what is > > > > > > > currently plugged in to validate the values? > > > > > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving > > > > > > DP > > > > > > or HDMI (with a passive dongle). > > > > > > Based on the type of sink detected, we should expose DP or > > > > > > HDMI > > > > > > colorspaces to userspace. > > > > > > > > > > For i915 DP connector always drives DP mode, HDMI connector > > > > > always > > > > > drives > > > > > HDMI mode, even when the physical connector is DP++. > > > > > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and > > > > amdgpu > > > > create 1 connector (not sure about other drivers) for a single > > > > physical DP++ socket. Since we supply the list of valid values at > > > > the > > > > time of creating the connector, we can't know at that point > > > > whether > > > > in > > > > the future a HDMI or DP will be plugged into it. > > > > > > > > -ilia > > > Ilia, does it mean that the drm_connector type is > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? > > > > That is correct. The connector type is "DisplayPort" in such a case. > > > > Cheers, > > > > -ilia > > For now drm_mode_create_colorspace_property() is only used for i915. > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for > using of drm_mode_create_colorspace_property(), > what about do we add a variable which can identify DP++ and DP to > drm_connector? > And when the drivers (nouveau, radeon, and amdgpu) detect the current > protocol, the drivers will set the variable. I've been working on adding this to nouveau. Can/should such properties be added/removed at "runtime", rather than at connector creation time? Either way, the function drm_mode_create_colorspace_property as proposed would not be reusable, since it looks at the connector type, which will always be "DisplayPort" in such cases. -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Nouveau] [Intel-gfx] [PATCH v6 08/17] drm/ttm: use gem vma_node
On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding wrote: > > On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote: > > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann wrote: > > > > > > Hi, > > > > > > > > Changing the order doesn't look hard. Patch attached (untested, have > > > > > no > > > > > test hardware). But maybe I missed some detail ... > > > > > > > > I came up with something very similar by splitting up nouveau_bo_new() > > > > into allocation and initialization steps, so that when necessary the GEM > > > > object can be initialized in between. I think that's slightly more > > > > flexible and easier to understand than a boolean flag. > > > > > > Yes, that should work too. > > > > > > Acked-by: Gerd Hoffmann > > Acked-by: Ben Skeggs > > Thanks guys, applied to drm-misc-next. Hi Thierry, Initial investigations suggest that this commit currently in drm-next commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5 Author: Thierry Reding Date: Wed Aug 14 11:00:48 2019 +0200 drm/nouveau: Initialize GEM object before TTM object breaks nouveau userspace which tries to allocate GEM objects with a non-page-aligned size. Previously nouveau_gem_new would just call nouveau_bo_init which would call nouveau_bo_fixup_align before initializing the GEM object. With this change, it is done after. What do you think -- OK to just move that bit of logic into the new nouveau_bo_alloc() (and make size/align be pointers so that they can be fixed up?) Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong wrote: > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote: > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä > > wrote: > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > > > > -Original Message- > > > > > From: Ilia Mirkin > > > > > Sent: Tuesday, September 3, 2019 6:12 PM > > > > > To: Mun, Gwan-gyeong > > > > > Cc: Intel Graphics Development > > > > >; Shankar, Uma > > > > > ; dri-devel < > > > > > dri-devel@lists.freedesktop.org> > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace > > > > > property > > > > > > > > > > So how would this work with a DP++ connector? Should it list > > > > > the HDMI or DP > > > > > properties? Or do we need a custom property checker which is > > > > > aware of what is > > > > > currently plugged in to validate the values? > > > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving DP > > > > or HDMI (with a passive dongle). > > > > Based on the type of sink detected, we should expose DP or HDMI > > > > colorspaces to userspace. > > > > > > For i915 DP connector always drives DP mode, HDMI connector always > > > drives > > > HDMI mode, even when the physical connector is DP++. > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and amdgpu > > create 1 connector (not sure about other drivers) for a single > > physical DP++ socket. Since we supply the list of valid values at the > > time of creating the connector, we can't know at that point whether > > in > > the future a HDMI or DP will be plugged into it. > > > > -ilia > Ilia, does it mean that the drm_connector type is > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode? That is correct. The connector type is "DisplayPort" in such a case. Cheers, -ilia ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä wrote: > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote: > > > > > > >-Original Message- > > >From: Ilia Mirkin > > >Sent: Tuesday, September 3, 2019 6:12 PM > > >To: Mun, Gwan-gyeong > > >Cc: Intel Graphics Development ; Shankar, > > >Uma > > >; dri-devel > > >Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property > > > > > >So how would this work with a DP++ connector? Should it list the HDMI or DP > > >properties? Or do we need a custom property checker which is aware of what > > >is > > >currently plugged in to validate the values? > > > > AFAIU For DP++ cases, we detect what kind of sink its driving DP or HDMI > > (with a passive dongle). > > Based on the type of sink detected, we should expose DP or HDMI colorspaces > > to userspace. > > For i915 DP connector always drives DP mode, HDMI connector always drives > HDMI mode, even when the physical connector is DP++. Right, i915 creates 2 connectors, while nouveau, radeon, and amdgpu create 1 connector (not sure about other drivers) for a single physical DP++ socket. Since we supply the list of valid values at the time of creating the connector, we can't know at that point whether in the future a HDMI or DP will be plugged into it. -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.
Signed-off-by: Ilia Mirkin
---
Only tested on G84 and GK208. The GV100+ is entirely untested.
With the fixed modetest tool, setting ilut and olut sizes to different
quantities seems to work out OK, across a range of formats (XR24, XB30,
XB4H).
drivers/gpu/drm/nouveau/dispnv50/base907c.c | 11 ---
drivers/gpu/drm/nouveau/dispnv50/head.c | 14 +-
drivers/gpu/drm/nouveau/dispnv50/head.h | 7 ---
drivers/gpu/drm/nouveau/dispnv50/head507d.c | 9 +++--
drivers/gpu/drm/nouveau/dispnv50/head827d.c | 1 +
drivers/gpu/drm/nouveau/dispnv50/head907d.c | 11 ---
drivers/gpu/drm/nouveau/dispnv50/head917d.c | 1 +
drivers/gpu/drm/nouveau/dispnv50/headc37d.c | 11 ---
drivers/gpu/drm/nouveau/dispnv50/headc57d.c | 12
drivers/gpu/drm/nouveau/dispnv50/lut.c | 2 +-
drivers/gpu/drm/nouveau/dispnv50/wndw.c | 17 -
drivers/gpu/drm/nouveau/dispnv50/wndw.h | 3 ++-
drivers/gpu/drm/nouveau/dispnv50/wndwc37e.c | 11 ---
drivers/gpu/drm/nouveau/dispnv50/wndwc57e.c | 11 +++
14 files changed, 84 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
index 5f2de77e0f32..224a34c340fe 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/base907c.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/base907c.c
@@ -75,12 +75,16 @@ 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)
+static bool
+base907c_ilut(struct nv50_wndw *wndw, struct nv50_wndw_atom *asyw, int size)
{
- asyw->xlut.i.mode = 7;
+ if (size != 256 && size != 1024)
+ return false;
+
+ asyw->xlut.i.mode = size == 1024 ? 4 : 7;
asyw->xlut.i.enable = 2;
asyw->xlut.i.load = head907d_olut_load;
+ return true;
}
static inline u32
@@ -160,6 +164,7 @@ base907c = {
.csc_set = base907c_csc_set,
.csc_clr = base907c_csc_clr,
.olut_core = true,
+ .ilut_size = 1024,
.xlut_set = base907c_xlut_set,
.xlut_clr = base907c_xlut_clr,
.image_set = base907c_image_set,
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 71c23bf1fe25..42ba68dadb80 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -235,9 +235,13 @@ nv50_head_atomic_check_lut(struct nv50_head *head,
return 0;
}
+ if (!head->func->olut(head, asyh, drm_color_lut_size(olut))) {
+ DRM_DEBUG_KMS("Invalid olut\n");
+ return -EINVAL;
+ }
asyh->olut.handle = disp->core->chan.vram.handle;
asyh->olut.buffer = !asyh->olut.buffer;
- head->func->olut(head, asyh);
+
return 0;
}
@@ -511,11 +515,11 @@ nv50_head_create(struct drm_device *dev, int index)
drm_crtc_init_with_planes(dev, crtc, >plane, >plane,
_head_func, "head-%d", head->base.index);
drm_crtc_helper_add(crtc, _head_help);
+ /* Keep the legacy gamma size at 256 to avoid compatibility issues */
drm_mode_crtc_set_gamma_size(crtc, 256);
- if (disp->disp->object.oclass >= GF110_DISP)
- drm_crtc_enable_color_mgmt(crtc, 256, true, 256);
- else
- drm_crtc_enable_color_mgmt(crtc, 0, false, 256);
+ drm_crtc_enable_color_mgmt(crtc, base->func->ilut_size,
+ disp->disp->object.oclass >= GF110_DISP,
+ head->func->olut_size);
if (head->func->olut_set) {
ret = nv50_lut_init(disp, >client.mmu, >olut);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.h
b/drivers/gpu/drm/nouveau/dispnv50/head.h
index d1c002f534d4..5c2cda2f32a8 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.h
@@ -20,8 +20,9 @@ void nv50_head_flush_clr(struct nv50_head *, struct
nv50_head_atom *, bool y);
struct nv50_head_func {
void (*view)(struct nv50_head *, struct nv50_head_atom *);
void (*mode)(struct nv50_head *, struct nv50_head_atom *);
- void (*olut)(struct nv50_head *, struct nv50_head_atom *);
+ bool (*olut)(struct nv50_head *, struct nv50_head_atom *, int);
bool olut_identity;
+ int olut_size;
void (*olut_set)(struct nv50_head *, struct nv50_head_atom *);
void (*olut_clr)(struct nv50_head *);
void (*core_calc)(struct nv50_head *, struct nv50_head_atom *);
@@ -43,7 +44,7 @@ struct nv50_head_func {
extern const struct nv50_head_func head507d;
void head507d_view
Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix
On Wed, Sep 4, 2019 at 4:08 PM Jyri Sarha wrote:
>
> On 04/09/2019 14:11, Laurent Pinchart wrote:
> > Hi Jyri,
> >
> > On Wed, Sep 04, 2019 at 10:17:00AM +0300, Jyri Sarha wrote:
> >> On 03/09/2019 18:24, Laurent Pinchart wrote:
> >>> On Mon, Sep 02, 2019 at 03:53:56PM +0300, Tomi Valkeinen wrote:
> From: Jyri Sarha
>
> Implement CTM color management property for OMAP CRTC using DSS
> overlay manager's Color Phase Rotation matrix. The CPR matrix does not
> exactly match the CTM property documentation. On DSS the CPR matrix is
> applied after gamma table look up. However, it seems stupid to add a
> custom property just for that.
> >>>
> >>> In that case the DRM documentation should be updated to mention that
> >>> both options are allowed.
> >>
> >> Ok, if that is alright. But if we do that, then I guess all the drivers
> >> implementing CTM should document the point where it is applied in the
> >> pipeline.
> >
> > Whatever solution we end up picking, I think it should at least be
> > discussed with a broader upstream audience and not just swept under the
> > omapdrm carpet :-)
> >
>
> I'll try to write something and send the next series to wider audience.
> Let's see what jury says.
In case it's useful ... the pipeline normally goes degamma -> ctm ->
gamma. If your ctm is applied after gamma, perhaps you can just rename
"gamma" to "degamma" and be done? (There's the unfortunate case of
legacy gamma which does end up in "GAMMA" when using atomic helpers.
But in such a case, you won't have a CTM.)
>
> Signed-off-by: Jyri Sarha
> Signed-off-by: Tomi Valkeinen
> ---
> drivers/gpu/drm/omapdrm/omap_crtc.c | 39 +++--
> 1 file changed, 37 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c
> b/drivers/gpu/drm/omapdrm/omap_crtc.c
> index 3c5ddbf30e97..d63213dd7d83 100644
> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c
> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
> @@ -391,6 +391,32 @@ static void omap_crtc_manual_display_update(struct
> work_struct *data)
> }
> }
>
> +static s16 omap_crtc_S31_32_to_s2_8(s64 coef)
> +{
> + uint64_t sign_bit = 1ULL << 63;
> + uint64_t cbits = (uint64_t) coef;
> >>>
> >>> s/uint64_t/u64/ for both lines as we're dealing with kernel code. And
> >>> there's no need for a space before coef.
> >>>
> + s16 ret = clamp_val(((cbits & ~sign_bit) >> 24), 0, 0x1FF);
> +
> + if (cbits & sign_bit)
> + ret = -ret;
> +
> + return ret;
> >>>
> >>> Can't this be simplified to
> >>>
> >>> s16 ret = (coef >> 24) & 0x1ff;
> >>>
> >>> return coef < 0 ? -ret : ret;
> >>>
> >>
> >> No. Clamping is different thing. If the original value is greater than
> >> what we can present with our 2 magnitude bit, we want to use the maximum
> >> value, not something that we may have in the LSB end of bits. e.g if
> >> user-space tries to set the value to 2.0 (= 0x200) we rather present it
> >> as 1.996 (= 0x1FF) than 0.0 (= 0x000).
> >
> > Of course, my bad.
> >
> > Perhaps a stupid question, should we reject out of range values at
> > atomic check time ?
> >
>
> I've at least seen CSC matrices with 2.0 values, so I think we should
> accept those and use clamping, but maybe we should reject CTMs with
> values far bigger than what we can represent. Such matrices would hardly
> work the way they were intended.
I clamped in nouveau. Not 100% sure it's the right policy. Having
something consistent across drivers is probably good. I don't think I
came up with clamping all by myself, so someone else must have been
doing it.
(https://cgit.freedesktop.org/drm/drm/commit/drivers/gpu/drm/nouveau?id=88b703527ba70659365d989f29579f1292ebf9c3
-- see csc_drm_to_base.)
Cheers,
-ilia
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
So how would this work with a DP++ connector? Should it list the HDMI
or DP properties? Or do we need a custom property checker which is
aware of what is currently plugged in to validate the values?
On Tue, Sep 3, 2019 at 5:12 AM Gwan-gyeong Mun
wrote:
>
> In order to use colorspace property to Display Port connectors, it extends
> DRM_MODE_CONNECTOR_DisplayPort connector_type on
> drm_mode_create_colorspace_property function.
>
> v3: Addressed review comments from Ville
> - Add new colorimetry options for DP 1.4a spec.
> - Separate set of colorimetry enum values for DP.
> v4: Add additional comments to struct drm_prop_enum_list.
> Polishing an enum string of struct drm_prop_enum_list
> Signed-off-by: Gwan-gyeong Mun
> Reviewed-by: Uma Shankar
> ---
> drivers/gpu/drm/drm_connector.c | 46 +
> include/drm/drm_connector.h | 8 ++
> 2 files changed, 54 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..5834e6d330a0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -882,6 +882,44 @@ static const struct drm_prop_enum_list
> hdmi_colorspaces[] = {
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
> };
>
> +/*
> + * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel
> Encoding/Colorimetry
> + * Format Table 2-120
> + */
> +static const struct drm_prop_enum_list dp_colorspaces[] = {
> + /* For Default case, driver will set the colorspace */
> + { DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
> + /* Colorimetry based on IEC 61966-2-1 */
> + { DRM_MODE_COLORIMETRY_SRGB, "sRGB" },
> + { DRM_MODE_COLORIMETRY_WIDE_GAMUT_FIXED_POINT_RGB,
> "wide_gamut_fixed_point_RGB" },
> + /* Colorimetry based on IEC 61966-2-2, wide gamut floating point RGB
> */
> + { DRM_MODE_COLORIMETRY_SCRGB, "scRGB" },
> + { DRM_MODE_COLORIMETRY_ADOBE_RGB, "Adobe_RGB" },
> + /* Colorimetry based on SMPTE RP 431-2 */
> + { DRM_MODE_COLORIMETRY_DCP_P3_RGB, "DCI-P3_RGB" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> + { DRM_MODE_COLORIMETRY_BT601_YCC, "BT601_YCC" },
> + { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" },
> + /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> + { DRM_MODE_COLORIMETRY_XVYCC_601, "XVYCC_601" },
> + /* High Definition Colorimetry based on IEC 61966-2-4 */
> + { DRM_MODE_COLORIMETRY_XVYCC_709, "XVYCC_709" },
> + /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
> + { DRM_MODE_COLORIMETRY_SYCC_601, "SYCC_601" },
> + /* Colorimetry based on IEC 61966-2-5 [33] */
> + { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> + /* Colorimetry based on ITU-R BT.2020 */
> + { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> + /*
> +* Colorumetry based on Digital Imaging and Communications in Medicine
> +* (DICOM) Part 14: Grayscale Standard Display Function
> +*/
> + { DRM_MODE_COLORIMETRY_DICOM_PART_14_GRAYSCALE,
> "DICOM_Part_14_Grayscale" },
> +};
> +
> /**
> * DOC: standard connector properties
> *
> @@ -1710,6 +1748,14 @@ int drm_mode_create_colorspace_property(struct
> drm_connector *connector)
> ARRAY_SIZE(hdmi_colorspaces));
> if (!prop)
> return -ENOMEM;
> + } else if (connector->connector_type ==
> DRM_MODE_CONNECTOR_DisplayPort ||
> + connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> + "Colorspace",
> + dp_colorspaces,
> + ARRAY_SIZE(dp_colorspaces));
> + if (!prop)
> + return -ENOMEM;
> } else {
> DRM_DEBUG_KMS("Colorspace property not supported\n");
> return 0;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8848e5d6b0c4 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -281,6 +281,14 @@ enum drm_panel_orientation {
> /* Additional Colorimetry extension added as part of CTA 861.G */
> #define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D6511
> #define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER12
> +/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
> +#define DRM_MODE_COLORIMETRY_SRGB 13
> +#define DRM_MODE_COLORIMETRY_WIDE_GAMUT_FIXED_POINT_RGB14
> +#define DRM_MODE_COLORIMETRY_SCRGB 15
Re: [Nouveau] [PATCH v3 5/7] drm/nouveau: utilize subconnector property for DP
On Wed, Aug 28, 2019 at 10:54 AM Ville Syrjälä wrote: > > On Wed, Aug 28, 2019 at 10:47:08AM -0400, Ilia Mirkin wrote: > > On Wed, Aug 28, 2019 at 10:38 AM Ville Syrjälä > > wrote: > > > > > > On Mon, Aug 26, 2019 at 09:36:50AM -0400, Ilia Mirkin wrote: > > > > 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.) > > > > > > One may or may not be able to detect it very well. I've seen dongles > > > that automagically change the DFP type from DP++ to DP/HDMI depending > > > on what's plugged in, and I've also seen dongles that leave the DFP > > > type to DP++. > > > > Well, our internal logic certainly knows if it's driving DP or TMDS. I > > just don't remember how it knows this. > > You'll be driving DP in this case. The DFP will be the one driving > DP or TMDS depending on what's plugged in. OK. That's not the case I was worried about though :) I'm worried about the case where you plug a true HDMI thing into the DP port, which will then be driven by HDMI and none of the dp.* things will be set (I don't remember, but they might even be in a union). I think Intel multiplies connectors for this, but nouveau/amd just have a single connector that handles both. -ilia ___ 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
On Wed, Aug 28, 2019 at 10:38 AM Ville Syrjälä wrote: > > On Mon, Aug 26, 2019 at 09:36:50AM -0400, Ilia Mirkin wrote: > > 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.) > > One may or may not be able to detect it very well. I've seen dongles > that automagically change the DFP type from DP++ to DP/HDMI depending > on what's plugged in, and I've also seen dongles that leave the DFP > type to DP++. Well, our internal logic certainly knows if it's driving DP or TMDS. I just don't remember how it knows this. -ilia ___ 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(>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 = _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
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: [PATCH 1/2] drm/nouveau/dispnv04: Grab/put runtime PM refs on DPMS on/off
On Wed, Aug 7, 2019 at 5:55 PM Daniel Vetter wrote:
>
> On Wed, Aug 07, 2019 at 05:33:00PM -0400, Lyude Paul wrote:
> > The code claims to grab a runtime PM ref when at least one CRTC is
> > active, but that's not actually the case as we grab a runtime PM ref
> > whenever a CRTC is enabled regardless of it's DPMS state. Meaning that
> > we can end up keeping the GPU awake when there are no screens enabled,
> > something we don't really want to do.
> >
> > Note that we fixed this same issue for nv50 a while ago in:
> >
> > commit e5d54f193572 ("drm/nouveau/drm/nouveau: Fix runtime PM leak in
> > nv50_disp_atomic_commit()")
> >
> > Since we're about to remove nouveau_drm->have_disp_power_ref in the next
> > commit, let's also simplify the RPM code here while we're at it: grab a
> > ref during a modeset, grab additional RPM refs for each CRTC enabled by
> > said modeset, and drop an RPM ref for each CRTC disabled by said
> > modeset. This allows us to keep the GPU awake whenever screens are
> > turned on, without needing to use nouveau_drm->have_disp_power_ref.
> >
> > Signed-off-by: Lyude Paul
> > ---
> > drivers/gpu/drm/nouveau/dispnv04/crtc.c | 18 --
> > 1 file changed, 4 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > index f22f01020625..08ad8e3b9cd2 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > @@ -183,6 +183,10 @@ nv_crtc_dpms(struct drm_crtc *crtc, int mode)
> > return;
> >
> > nv_crtc->last_dpms = mode;
> > + if (mode == DRM_MODE_DPMS_ON)
> > + pm_runtime_get_noresume(dev->dev);
> > + else
> > + pm_runtime_put_noidle(dev->dev);
>
> it's after we filter out duplicate operations, so that part looks good.
> But not all of nouveau's legacy helper crtc callbacks go throuh ->dpms I
> think: nv_crtc_disable doesn't, and crtc helpers use that in preference
> over ->dpms in some cases.
>
> I think the only way to actually hit that path is if you switch an active
> connector from an active CRTC to an inactive one. This implicitly disables
> the crtc (well, should, nv_crtc_disable doesn't seem to shut down hw), and
> I think would leak your runtime PM reference here. At least temporarily.
>
> No idea how to best fix that. Aside from "use atomic" :-)
Not sure if this is relevant to the discussion at hand, but I'd like
to point out that dispnv04 is for pre-nv50 things, which definitely
didn't support any kind of ACPI-based runtime suspend.
-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 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, _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: [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 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: [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
[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: [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(>modes[j]);
> + dump_mode(>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(>mode);
> + dump_mode(>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 >modes[index];
> + }
> +
> + /* Pick by Name */
> for (i = 0; i < connector->count_modes; i++) {
> mode = >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: [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(>modes[j]);
> + dump_mode(>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(>mode);
> + dump_mode(>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 >modes[index];
> + }
> +
> + /* Pick by Name */
> for (i = 0; i < connector->count_modes; i++) {
> mode = >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
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 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 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
[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, >plane, >plane,
_head_func, "head-%d", head->base.index);
drm_crtc_helper_add(crtc, _head_help);
- drm_mode_crtc_set_gamma_size(crtc, 256);
+ drm_mode_crtc_set_gamma_size(crtc, head->func->lut_size);
if (disp->disp->object.oclass >= GF110_DISP &&
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
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: [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(>reg_lock);
> + VOP_REG_SET(vop, common, dsp_lut_en, 0);
> + vop_cfg_done(vop);
> + spin_unlock(>reg_lock);
> +
> + ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop,
> + idle, !idle, 5, 30 * 1000);
> + if (ret)
> + return;
> +
> + spin_lock(>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(>reg_lock);
> +}
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: [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, >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, >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(>reg_lock);
> + VOP_REG_SET(vop, common, dsp_lut_en, 0);
> + vop_cfg_done(vop);
> + spin_unlock(>reg_lock);
> +
> + ret = readx_poll_timeout(vop_dsp_lut_is_enable, vop,
> + idle, !idle, 5, 10 * 3);
> + if (ret)
> + return;
> +
> + spin_lock(>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(>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(>fb_unref_work, old_plane_state->fb);
> set_bit(VOP_PENDING_FB_UNREF, >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, _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 address after
> +* the MMU registers,
