Re: [PATCH v6 04/20] drm/imagination/uapi: Add PowerVR driver UAPI

[Adam Jackson]

On Wed, Sep 6, 2023 at 5:57 AM Sarah Walker  wrote:

>
> + *:BYPASS_CACHE: There are very few situations where this flag is
> useful.
> + *   By default, the device flushes its memory caches after every job.
>

Presumably BYPASS_CACHE does something other than "after every job". Is
that "never" or something else? Would be good if the comment was explicit.

- ajax

Re: [PATCH] char/agp: Disable frontend without CONFIG_DRM_LEGACY

[Adam Jackson]

On Tue, Nov 17, 2020 at 4:40 PM Daniel Vetter  wrote:
>
> It's probably full of bugs ready for exploiting by userspace. And
> there's not going to be any userspace for this without any of the drm
> legacy drivers enabled too. So just couple it together.

Not quite true. The only UMS driver using this is i810, which needs it
even with DRI disabled. I have difficulty caring though.

In related news, since i810 is only ever attached to 32-bit x86 we
could disable the frontend unconditionally on amd64 (and everything
else tbh).

Acked-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Mesa-dev] [Intel-gfx] gitlab.fd.o financial situation and impact on services

[Adam Jackson]

On Sat, 2020-04-04 at 08:11 -0700, Rob Clark wrote:
> On Fri, Apr 3, 2020 at 7:12 AM Michel Dänzer  wrote:
> > On 2020-03-01 6:46 a.m., Marek Olšák wrote:
> > > For Mesa, we could run CI only when Marge pushes, so that it's a strictly
> > > pre-merge CI.
> > 
> > Thanks for the suggestion! I implemented something like this for Mesa:
> > 
> > https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4432
> 
> I wouldn't mind manually triggering pipelines, but unless there is
> some trick I'm not realizing, it is super cumbersome.  Ie. you have to
> click first the container jobs.. then wait.. then the build jobs..
> then wait some more.. and then finally the actual runners.  That would
> be a real step back in terms of usefulness of CI.. one might call it a
> regression :-(

I think that's mostly a complaint about the conditionals we've written
so far, tbh. As I commented on the bug, when I clicked the container
job (which the rules happen to have evaluated to being "manual"), every
job (recursively) downstream of it got enqueued, which isn't what
you're describing. So I think if you can describe the UX you'd like we
can write rules to make that reality.

But I don't really know which jobs are most expensive in terms of
bandwidth, or storage, or CPUs, and even if I knew those I don't know
how to map those to currency. So I'm not sure if the UI we'd like would
minimize the cost the way we'd like.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: KMS enums and bitfields UAPI

[Adam Jackson]

On Fri, 2020-04-03 at 15:24 +0300, Pekka Paalanen wrote:
> On Fri, 03 Apr 2020 10:15:21 + Simon Ser  wrote:
> 
> > Additionally, I've heard Pekka saying that it would be nice to have 
> > constants
> > for property names in the UAPI headers. Indeed, this would prevent
> > hard-to-debug typo issues. I have a very good example of such a typo issue 
> > that
> > took literally hours to debug, with X11 atoms which also use free-form 
> > strings
> > like KMS properties [3].
> > 
> > If we have constants for property names in UAPI headers, it wouldn't be a 
> > big
> > hurdle to also have constants for enum values alongside.
> 
> To clarify, the property names would be of the form
> 
> #define DRM_KMS_PROPERTY_KERBLAH "KerBlah"
> 
> while enum values would be integers, i.e. the raw values.
> 
> Daniel Stone did have a good counter-argument to defining property
> names: userspace would be full of
> 
> #ifndef DRM_KMS_PROPERTY_KERBLAH
> #define DRM_KMS_PROPERTY_KERBLAH "KerBleh"
> #endif
> 
> anyway as long as they cannot rely on the headers to be recent enough.
> (The typo is intentional.)

Why not put this in some external registry and regularly sync it into
drm-next? This seems like an xorgproto kind of problem to me.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/2] drm: error out with EBUSY when device has existing master

[Adam Jackson]

On Thu, 2020-03-19 at 17:29 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> As requested by Adam, provide different error message for when the
> device has an existing master. An audit of the following projects, shows
> that the errno is used only for printf() purposes.
> 
> xorg/xserver
> xorg/drivers/xf86-video-ati
> xorg/drivers/xf86-video-amdgpu
> xorg/drivers/xf86-video-intel
> xorg/drivers/xf86-video-tegra
> xorg/drivers/xf86-video-freedreno
> xorg/drivers/xf86-video-nouveau
> xorg/drivers/xf86-video-vmwgfx
> 
> qt/kwin/plasma
> gtk/mutter/gnomeshell
> efl/enlightment
> 
> Cc: Adam Jackson 
> Suggested-by: Adam Jackson 
> Signed-off-by: Emil Velikov 

Delightful! Series is:

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Mesa-dev] Plumbing explicit synchronization through the Linux ecosystem

[Adam Jackson]

On Tue, 2020-03-17 at 10:12 -0700, Jacob Lifshay wrote:
> One related issue with explicit sync using sync_file is that combined
> CPUs/GPUs (the CPU cores *are* the GPU cores) that do all the
> rendering in userspace (like llvmpipe but for Vulkan and with extra
> instructions for GPU tasks) but need to synchronize with other
> drivers/processes is that there should be some way to create an
> explicit fence/semaphore from userspace and later signal it. This
> seems to conflict with the requirement for a sync_file to complete in
> finite time, since the user process could be stopped or killed.

DRI3 (okay, libxshmfence specifically) uses futexes for this. Would
that work for you? IIRC the semantics there are that if the process
dies the futex is treated as triggered, which seems like the only
sensible thing to do.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: rework SET_MASTER and DROP_MASTER perm handling

[Adam Jackson]

On Wed, 2020-02-19 at 13:27 +, Emil Velikov wrote:

> + * As some point systemd-logind was introduced to orchestrate and delegate
> + * master as applicable. It does so by opening the fd and passing it to users
> + * while in itself logind a) does the set/drop master per users' request and
> + * b)  * implicitly drops master on VT switch.
> + *
> + * Even though logind looks like the future, there are a few issues:
> + *  - using it is not possible on some platforms
> + *  - applications may not be updated to use it,
> + *  - any client which fails to drop master* can DoS the application using
> + * logind, to a varying degree.

I'm not super worried. Everything about VTs is a pile of DoS scenarios
that userspace has to dance to avoid. It sounds like this is only
introducing new DoS scenarios for cases that previously simply did not
work.

> + * As a result this fixes, the following when using root-less build w/o 
> logind

Nitpick: no comma here.

>  int drm_setmaster_ioctl(struct drm_device *dev, void *data,
>   struct drm_file *file_priv)
>  {
>   int ret = 0;
>  
>   mutex_lock(&dev->master_mutex);
> +
> + ret = drm_master_check_perm(dev, file_priv);
> + if (ret)
> + goto out_unlock;
> +
>   if (drm_is_current_master(file_priv))
>   goto out_unlock;

I'd mentioned this on IRC, and it doesn't need to be changed with this
patch, but it would be cool if the "does the device already have a
master" check just below here would return -EBUSY instead of -EINVAL so
userspace diagnostics have a chance of saying something useful. A quick
audit of the X drivers and weston shows no cases where we care about
the generated errno value beyond feeding it into strerror() so that
should also be safe.

Looks good otherwise, and these are definitely more reasonable
semantics. Thanks for taking this on!

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Plumbing explicit synchronization through the Linux ecosystem

[Adam Jackson]

On Wed, 2020-03-11 at 12:31 -0500, Jason Ekstrand wrote:

>  - X11: With present, it has these "explicit" fence objects but
> they're always a shmfence which lets the X server and client do a
> userspace CPU-side hand-off without going over the socket (and
> round-tripping through the kernel).  However, the only thing that
> fence does is order the OpenGL API calls in the client and server and
> the real synchronization is still implicit.

I'm pretty sure "the only thing that fence does" is an implementation
detail. PresentPixmap blocks until the wait-fence signals, but when and
how it signals are properties of the fence itself. You could have drm
give the client back a fence fd, pass that to xserver to create a fence
object, and name that in the PresentPixmap request, and then drm can do
whatever it wants to signal the fence.

> From my perspective, as a Vulkan driver developer, I have to deal with
> the fact that Vulkan is an explicit sync API but Wayland and X11
> aren't.

I'm quite sure we can give you an explicit-sync X11 API. I think you
may already have one.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2,1/3] drm/dp: Introduce EDID-based quirks

[Adam Jackson]

On Tue, 2020-02-11 at 13:33 -0500, Lyude Paul wrote:
> The whole point of using OUIs is so that we can recognize certain
> devices and potentially apply quirks for them. Normally this should work
> quite well, but there appears to be quite a number of laptop panels out
> there that will fill the OUI but not the device ID. As such, for devices
> like this I can't imagine it's a very good idea to try relying on OUIs
> for applying quirks. As well, some laptop vendors have confirmed to us
> that their panels have this exact issue.
> 
> So, let's introduce the ability to apply DP quirks based on EDID
> identification. We reuse the same quirk bits for OUI-based quirks, so
> that callers can simply check all possible quirks using
> drm_dp_has_quirk().

With the bug URL fixed in 2/3, series is:

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [v2,2/3] drm/i915: Force DPCD backlight mode on X1 Extreme 2nd Gen 4K AMOLED panel

[Adam Jackson]

On Tue, 2020-02-11 at 13:33 -0500, Lyude Paul wrote:

> - if (!intel_dp_aux_display_control_capable(intel_connector))
> + /*
> +  * There are a lot of machines that don't advertise the backlight
> +  * control interface to use properly in their VBIOS, :\
> +  */
> + if (dev_priv->vbt.backlight.type !=
> + INTEL_BACKLIGHT_VESA_EDP_AUX_INTERFACE &&
> + !drm_dp_has_quirk(&intel_dp->desc, intel_dp->edid_quirks,
> +   DP_QUIRK_FORCE_DPCD_BACKLIGHT)) {
> + DRM_DEV_INFO(dev->dev,
> +  "Panel advertises DPCD backlight support, but "
> +  "VBT disagrees. If your backlight controls "
> +  "don't work try booting with "
> +  "i915.enable_dpcd_backlight=1. If your machine "
> +  "needs this, please file a _new_ bug report on "
> +  "bugs.freedesktop.org against DRI -> "
> +  "DRM/Intel\n");

Bugzilla's been put out of our misery, probably this should point to
gitlab instead.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/fourcc: Fix undefined left shift in DRM_FORMAT_BIG_ENDIAN macros

[Adam Jackson]

1<<31 is undefined because it's a signed int and C is terrible.

Reviewed-by: Eric Engestrom 
Signed-off-by: Adam Jackson 
---
 include/uapi/drm/drm_fourcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 3feeaa3f987a..c06d34559fab 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -69,7 +69,7 @@ extern "C" {
 #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
 ((__u32)(c) << 16) | ((__u32)(d) << 24))
 
-#define DRM_FORMAT_BIG_ENDIAN (1<<31) /* format is big endian instead of 
little endian */
+#define DRM_FORMAT_BIG_ENDIAN (1U<<31) /* format is big endian instead of 
little endian */
 
 /* Reserve 0 for the invalid format specifier */
 #define DRM_FORMAT_INVALID 0
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/fourcc: Fix undefined left shift in DRM_FORMAT_BIG_ENDIAN macros

[Adam Jackson]

1<<31 is undefined because it's a signed int and C is terrible.

Reviewed-by: Eric Engestrom 
---
 include/uapi/drm/drm_fourcc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 3feeaa3f987a..c06d34559fab 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -69,7 +69,7 @@ extern "C" {
 #define fourcc_code(a, b, c, d) ((__u32)(a) | ((__u32)(b) << 8) | \
 ((__u32)(c) << 16) | ((__u32)(d) << 24))
 
-#define DRM_FORMAT_BIG_ENDIAN (1<<31) /* format is big endian instead of 
little endian */
+#define DRM_FORMAT_BIG_ENDIAN (1U<<31) /* format is big endian instead of 
little endian */
 
 /* Reserve 0 for the invalid format specifier */
 #define DRM_FORMAT_INVALID 0
-- 
2.23.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: customize DPCD brightness control for specific panel

[Adam Jackson]

On Mon, 2019-10-07 at 12:08 +0300, Jani Nikula wrote:

> The problem with the EDID quirks is that exposing the quirks sticks out
> like a sore thumb. Thus far all of it has been contained in drm_edid.c
> and they affect how the EDID gets parsed, for all drivers. Obviously
> this could be changed, but is it the right thing to do?
> 
> What I suggested was, check the OUI only, and if it matches, do
> more. Perhaps there's something in the 0x300 range of DPCD offsets that
> you can read? Or perhaps you need to write the source OUI first, and
> then do that.

My issue isn't really with identifying the panel from EDID rather than
DPCD, whichever identifier is most specific is probably the best thing
to use. It's more that this quirk is identified in common code but only
applied in one driver. If this panel were ever to be attached to some
other source, they might well want to apply the same kind of fix. My
(admittedly naïve) reading of the OUI handshake process is that when
the source device writes an OUI to DP_SOURCE_OUI it is telling the sink
"I'm about to issue commands that conform to _this_ vendor's own
conventions". If that convention communicates information that is
entirely contained within AUXCH transactions (and doesn't, for example,
require looking at some other strapping pin or external device) then in
principle it doesn't matter if the source device "matches" that OUI; it
would be legal for an AMD GPU to write the same sequence and expect the
same reaction, should that panel be attached to an AMD GPU.

So, it would be nice to know exactly what that protocol is meant to do,
if it applies only to this specific panel or anything else with the
same TCON, how one would identify such TCONs in the wild other than
EDID, if it relies on an external PWM or something, etc. And it might
make sense for now to make this a (shudder) driver-specific EDID quirk
rather than match by DPCD, at least until we know if the panel is ever
seen attached to other source devices and if the OUI convention is
self-contained.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: customize DPCD brightness control for specific panel

[Adam Jackson]

On Sat, 2019-10-05 at 05:58 +0800, Lee Shawn C wrote:
> This panel (manufacturer is SDC, product ID is 0x4141)
> used manufacturer defined DPCD register to control brightness
> that not defined in eDP spec so far. This change follow panel
> vendor's instruction to support brightness adjustment.

I'm sure this works, but this smells a little funny to me.

> + /* Samsung eDP panel */
> + { "SDC", 0x4141, EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL },

It feels a bit like a layering violation to identify eDP behavior
changes based on EDID. But I'm not sure there's any obvious way to
identify this device by its DPCD. The Sink OUI (from the linked
bugzilla) seems to be 0011F8, which doesn't match up to anything in my
oui.txt...

> @@ -1953,6 +1956,7 @@ static u32 edid_get_quirks(const struct edid *edid)
>  
>   return 0;
>  }
> +EXPORT_SYMBOL(edid_get_quirks);

If we're going to export this it should probably get a drm_ prefix.

> +#define DPCD_EDP_GETSET_CTRL_PARAMS  0x344
> +#define DPCD_EDP_CONTENT_LUMINANCE   0x346
> +#define DPCD_EDP_PANEL_LUMINANCE_OVERRIDE0x34a
> +#define DPCD_EDP_BRIGHTNESS_NITS 0x354
> +#define DPCD_EDP_BRIGHTNESS_OPTIMIZATION 0x358
> +
> +#define EDP_CUSTOMIZE_MAX_BRIGHTNESS_LEVEL   (512)

This also seems a bit weird, the 0x300-0x3FF registers belong to the
_source_ DP device. But then later...

> + /* write source OUI */
> + write_val[0] = 0x00;
> + write_val[1] = 0xaa;
> + write_val[2] = 0x01;

Oh hey, you're writing (an) Intel OUI to the Source OUI, so now it
makes sense that you're writing to registers whose behavior the source
defines. But this does raise the question: is this just a convention
between Intel and this particular panel? Would we expect this to work
with other similar panels? Is there any way to know to expect this
convention from DPCD instead?

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: image size is wrong in EDID, how to use EDID quirks

[Adam Jackson]

On Fri, 2019-09-13 at 04:21 +, zhangn1...@outlook.com wrote:
> Dear developers
>  
> I have a Samsung 40’ HDMI TV, which has wrong EDID.
>  
> The actaul size of this TV is 40’ (88cm*49cm), but in EDID the size
> is 49’ (106*63cm)
>  
> Thus makes image size is larger than screen, both in console and
> desktop.

That's not how EDID works in Linux. It's only used to compute the DPI
of the screen, not to scale the image. If the image on the TV looks
like it extends off the edge of the TV you need to find the "overscan"
setting in your TV and turn it off.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] RFT: drm/pl111: Support grayscale

[Adam Jackson]

On Tue, 2019-07-23 at 15:37 +0200, Linus Walleij wrote:
> Migrating the TI nspire calculators to use the PL111 driver for
> framebuffer requires grayscale support for the elder panel
> which uses 8bit grayscale only.
> 
> DRM does not support 8bit grayscale framebuffers in memory,
> but by defining the bus format to be MEDIA_BUS_FMT_Y8_1X8 we
> can get the hardware to turn on a grayscaling feature and
> convert the RGB framebuffer to grayscale for us.

What's wrong with DRM_FORMAT_R8? Yes the hardware is not really
"redscale", but it's still a single color channel and there's not
really any ambiguity.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: dw-hdmi: Refuse DDC/CI transfers on the internal I2C controller

[Adam Jackson]

On Thu, 2019-07-18 at 14:41 -0700, Matthias Kaehlcke wrote:

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 045b1b13fd0e..e49402ebd56f 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -35,6 +35,7 @@
>  
>  #include 
>  
> +#define DDC_I2C_ADDR 0x37

This confused the heck out of me to read, DDC by definition happens
over I2C and this one address is just for a specific subset of DDC.
Perhaps this would be clearer if it was named DDC_CI_ADDR.

- ajax

Re: [PATCH] drm/cirrus: rewrite and modernize driver.

[Adam Jackson]

On Wed, 2019-04-03 at 16:15 +0100, Daniel Stone wrote:

> There's already a list of supported formats for each DRM plane, which
> you can get via drmModeGetPlane (being careful to enable universal
> planes so you can discover the primary plane). The same information is
> present in the 'IN_FORMATS' property, which is more difficult to parse
> but also tells you about modifiers.
> 
> modesetting already pulls all this out (at least in the atomic path)
> so we can reason about acceptable modifiers.

D'oh, I knew that. The problem then is modesetting isn't pulling that
info out early enough in PreInit. I'll consider that another case of
xorg/xserver#9 then.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/cirrus: rewrite and modernize driver.

[Adam Jackson]

On Wed, 2019-04-03 at 09:23 +0200, Gerd Hoffmann wrote:

>  - Only DRM_FORMAT_RGB565 (depth 16) is supported.  The old driver does
>that too by default.  There was a module parameter which enables 24/32
>bpp support and disables higher resolutions (due to cirrus hardware
>constrains).  That parameter wasn't reimplemented.

One slightly annoying aspect of this (well, initially of the patch to
clamp the default to 16bpp, but this too) is that we only have a way to
ask the driver which format it prefers, not which ones it supports at
all. For X's modesetting driver (and yes some of this is because X is
awful) this creates the following failure mode:

1: user sets up xorg.conf for depth 24
2: user upgrades kernel, reboots
3: X driver detects that depth 16 is preferred, but
4: X core respects user's xorg.conf and tries depth 24, which
5: throws -EINVAL and X won't start.

Possibly X should work around this by transparently setting up a shadow
framebuffer at the user's requested depth. The problem there is, if 565
is preferred but  works, you're adding a format-conversion blit in
the middle for no reason. If I could ask the kernel for the entire list
of supported formats, I could only set up the shadow if it was
necessary.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/11] drm/fbdevdrm: Add fbdevdrm device

[Adam Jackson]

On Tue, 2019-03-26 at 10:17 +0100, Thomas Zimmermann wrote:

> +static bool is_generic_driver(const struct fb_info *fb_info)
> +{
> + /* DRM porting note: We don't want to bind to vga16fb, vesafb, or any
> +  * other generic fbdev driver. Usually, these drivers have limited
> +  * capabilitis. We only continue if the fix structure indicates a
> +  * hardware-specific drivers . This test will also sort out drivers
> +  * registered via DRM's fbdev emulation. If you're porting an fbdev
> +  * driver to DRM, you can remove this test. The module's PCI device
> +  * ids will contain this information.
> +  */
> + return !fb_info->fix.accel &&
> +!!strcmp(fb_info->fix.id, "S3 Virge/DX");
> +}

This seems odd. s3fb sets fix.accel to NULL unconditionally AFAICT, not
sure why you're testing for that explicitly.

I do have a question though: why _not_ support generic fbdev drivers?
If I had that, and the ability to disable creation of /dev/fb*, I could
expose a consistent video device enumeration to userspace. As it stands
I have no reasonable way of knowing which fbdev and drm devices are
pointed at the same hardware. If there were only drm devices...

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/edid: Remove defunct EDID_QUIRK_FIRST_DETAILED_PREFERRED

[Adam Jackson]

On Fri, 2019-03-22 at 19:42 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Looks like EDID_QUIRK_FIRST_DETAILED_PREFERRED never did anything.
> Its counterpart in f86EdidModes.c is properly hooked up but somehow
> that functionality was lost when it was copied into the kernel.
> 
> The concensus seems to be that this quirk is a bit misguided
> anyway so let's nuke the leftovers.

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/fourcc: Fix conflicting Y41x definitions

[Adam Jackson]

On Tue, 2019-03-19 at 13:17 +0100, Maarten Lankhorst wrote:
> There has unfortunately been a conflict with the following 3 commits:
> 
> commit e9961ab95af81b8d29054361cd5f0c575102cf87
> Author: Ayan Kumar Halder 
> Date:   Fri Nov 9 17:21:12 2018 +
> drm: Added a new format DRM_FORMAT_XVYU2101010
> 
> commit 7ba0fee247ee7a36b3bfbed68f6988d980aa3aa3
> Author: Brian Starkey 
> Date:   Fri Oct 5 10:27:00 2018 +0100
> 
> drm/fourcc: Add AFBC yuv fourccs for Mali
> 
> and
> 
> commit 50bf5d7d595fd0705ef3785f80e679b6da501e5b
> Author: Swati Sharma 
> Date:   Mon Mar 4 17:26:33 2019 +0530
> 
> drm: Add Y2xx and Y4xx (xx:10/12/16) format definitions and fourcc
> 
> Unfortunately gcc didn't warn about the redefinitions, because the
>

... sentence got cut off here.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PULL] topic/hdr-formats

[Adam Jackson]

On Mon, 2019-03-11 at 12:19 +0100, Maarten Lankhorst wrote:
> Hey,
> 
> Op 07-03-2019 om 18:12 schreef Adam Jackson:
> > On Thu, 2019-03-07 at 10:48 +0100, Maarten Lankhorst wrote:
> > > Hi Sean and Joonas,
> > > 
> > > Here's a pull request for HDR format enabling in i915. Can this be pulled 
> > > to drm-misc-next and dinq?
> > Could you also add Kevin Strasser's patch for FP16 formats? For that
> > matter I'd like to see FP32 added too, but I don't think there's been a
> > patch sent for that yet.
> 
> Added kevin to CC.
> 
> Is the mesa side considered completely reviewed then?

Is that strictly necessary for just adding a fourcc? I get wanting to
see a real userspace consumer first, but this is just allocating some
magic numbers not promising that a certain ioctl behaves a certain way.

That said, I did say r-b for the Mesa series as a whole. Daniel Stone
had some comments on 13/13 that made it sound like it (that specific
patch) was unnecessary/undesirable, but everything before that looked
great to me, and even if we only merged the first 12 we'd still have
FP16 wired through for the gbm EGL platform, meaning HDR scanout could
actually be a thing.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PULL] topic/hdr-formats

[Adam Jackson]

On Thu, 2019-03-07 at 10:48 +0100, Maarten Lankhorst wrote:
> Hi Sean and Joonas,
> 
> Here's a pull request for HDR format enabling in i915. Can this be pulled to 
> drm-misc-next and dinq?

Could you also add Kevin Strasser's patch for FP16 formats? For that
matter I'd like to see FP32 added too, but I don't think there's been a
patch sent for that yet.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data

[Adam Jackson]

On Tue, 2019-03-05 at 09:08 +0100, Maxime Ripard wrote:

> Chances are that they would stop at 1, call the device trash and never
> submit any quirk, therefore making the quirk approach useless in the
> process.

But the device _is_ trash. It's not like writing an accurate EDID is
difficult, if you're the manufacturer. Why enable hardware vendors to
be incompetent?

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/7] drm/edid: Allow to ignore the audio EDID data

[Adam Jackson]

On Mon, 2019-03-04 at 17:47 +0200, Jani Nikula wrote:
> On Mon, 04 Mar 2019, Maxime Ripard  wrote:
> > In some cases, in order to accomodate with displays with poor EDIDs, we
> > need to ignore that the monitor alledgedly supports audio output and
> > disable the audio output.
> 
> *sad trombone*
> 
> Trying to figure this out automatically in kernel is better than a
> quirk.
> 
> A quirk is better than requiring the user to provide an override EDID
> via the firmware loader (drm.edid_firmware parameter).
> 
> Requiring an override EDID is better than adding a module parameter.

Also, this and 3/ apply to _every_ monitor attached to the system. No.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/edid: If no preferred mode is found assume the first mode is preferred

[Adam Jackson]

On Wed, 2019-02-27 at 23:03 +0200, Ville Syrjälä wrote:

> So instead of putting this logic into the EDID parser I guess we
> could just put it into the i915 fixed mode code. But then I suppose
> we should also fix EDID_QUIRK_FIRST_DETAILED_PREFERRED (assuming it
> exists for a good reason).

I don't think it has any good reason to exist, tbh. The commit adding
it to xserver was for the Philips 107P5, which - being a CRT - would be
entirely expected to not have the preferred bit set. We should probably
have handled that instead by letting the "target a DPI near 96" logic
handle things.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm/edid: If no preferred mode is found assume the first mode is preferred

[Adam Jackson]

On Wed, 2019-02-27 at 19:14 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Some monitors apparently forget to mark any mode as preferred in the
> EDID. In this particular case we have a very generic looking ID
> "PNP Model 0 Serial Number 4" / "LVDS 800x600" so a specific quirk
> doesn't seem particularly wise. Also the quirk we have
> (EDID_QUIRK_FIRST_DETAILED_PREFERRED) is actually defunct so we'd
> have to fix it first.
>
> As a generic fallback let's just mark the first probed mode (which
> should be the first detailed mode, assuming there are any) as
> preferred.

What problem is this trying to fix? Userspace (and drm for that matter)
is typically going to pick the first mode in the list anyway if there's
none marked as preferred. Not having any preferred modes was pretty
common on CRTs IIRC.

The other major case I've seen of a monitor with no preferred mode are
the early dual-link DVI displays without internal scalers (Apple
Cinema, Dell 3007WFP, etc). You end up with 1280x800 first in the list
since 2560x1600 doesn't fit in a single DVI link. It might be nice if
such monitors decided their preferred mode based on the link
capabilities; if you know it's a dual-link capable port, you'd probably
prefer 2560x1600.

Does it make more sense to run the "infer a preferred mode" logic after
we've done mode validation for the output?

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/3] Support 64 bpp half float formats

[Adam Jackson via dri-devel]

On Fri, 2019-02-08 at 13:49 -0800, Kevin Strasser wrote:
> This series defines new formats and adds implementation to the i915 driver.
> Since posting v1 I have removed the pixel normalize property, as it's not 
> needed
> for basic functionality. Also, I have been working on adding support to
> userspace, but we can't land any patches until drm_fourcc.h has been updated
> here.

Series is:

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Expose more EDID fields to userspace

[Adam Jackson]

On Wed, 2019-01-16 at 20:35 +0200, Pekka Paalanen wrote:

> I would agree with an effort to establish a userspace EDID parsing
> library in any case. As mentioned above, there will probably be too
> much to expose via kernel UABI, or it will become just another
> encoded format that again should have a shared parser library in
> userspace.
> 
> Would it be possible to architect the library so that it would be
> shared with the kernel? Maybe the quirks database could be shared
> with the kernel as well? That way both kernel and userspace would
> more or less agree on the parsing details.

If the kernel wanted to expose its quirks db somehow the library could
certainly be taught to use it. However there are likely to be quirks
relevant only to userspace (see below), making the kernel carry that
doesn't make a ton of sense.

> I've been dreaming of a "liboutput" that would e.g. parse EDID,
> generate CVT video mode timings, and whatnot that all display
> servers more or less will duplicate. Once upon a time Ajax started
> minitru IIRC...

It's still not the worst idea, I'd be happy to revisit it.

Part of the problem with the idea is that EDID parsing is not
unambiguous. There are several fields for "physical output size" with
slightly different semantics, which one do you want? There are both
structured and free-form ways to encode monitor name and serial number.
There are multiple ways to ask for 1920x1080, how many slightly
different timings do you want? Some more-modern displays have DisplayID
blocks too, which we're not fetching, and which can easily disagree
with EDID; which version of reality would you like?

Me personally I'm happy to make policy decisions about this kind of
thing, but there are likely to be cases where a display server wants to
make more subtle decisions, in which case it's likely to ignore any
"cooked" queries you might of the library and instead try parsing the
data itself. Many of these decisions currently don't affect the kernel
at all (we don't use physical size for anything, but userspace thinks
it wants to know about DPI...).

> Another thing for "liboutput" was a device description database,
> whose first use would have been the "non-desktop" property. Because
> we don't expose monitors through udev to have udev rules tag them
> with interesting bits.
> 
> Imagine this: monitors exposed as devices via udev, tagged with
> helpers as regular monitor, HMD, TV, projector, ... and all EDID
> fields decoded as well. And quirks in hwdb. But I suppose that
> won't happen, because a "monitor device node" would have no other
> use. Except... programmatical monitor controls? Like backlight,
> brightness, contrast, and so on.

This could be interesting. I'd half-written DDC/CI support for drm a
while ago, which would allow us to expose that kind of thing. But to be
broadly useful you'd want to mirror the same kind of knobs for like
platform backlight control, and so far the kernel has steadfastly
refused to believe it can possibly associate a backlight controller
with a drm output in any userspace-useful way.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] RFC: Migration to Gitlab

[Adam Jackson]

On Wed, 2018-08-22 at 16:13 +0300, Jani Nikula wrote:

> - Sticking to fdo bugzilla and disabling gitlab issues for at least
>   drm-intel for the time being. Doing that migration in the same go is a
>   bit much I think. Reassignment across bugzilla and gitlab will be an
>   issue.

Can you elaborate a bit on the issues here? The actual move-the-bugs
process has been pretty painless for the parts of xorg we've done so
far.

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3] drm/cirrus: flip default from 24bpp to 16bpp

[Adam Jackson]

On Wed, 2018-08-08 at 13:13 +0200, Gerd Hoffmann wrote:
> The problem with 24bpp is that it is a rather unusual depth these days,

Nit: s/depth/bpp/ here. Depth 24 is very common, it's packing that into
three bytes per pixel that's unusual.

> cirrus is pretty much the only relevant device still using that, and it
> is a endless source of issues.  Wayland doesn't support it at all.  Bugs
> in Xorg keep showing up.
> 
> Typically either 32bpp or 16bpp are used.  Using 32bpp would limit the
> resolution to 800x600 due to hardware constrains.  So lets go with 16bpp.
> 
> Also use the default depth for the framebuffer console and
> mode_info->preferred_depth.
> 
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: Adam Jackson 

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC 1/3] drm: Add colorspace property

[Adam Jackson]

On Tue, 2018-07-24 at 21:15 +0530, Uma Shankar wrote:

> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -209,6 +209,17 @@
>  #define DRM_MODE_CONTENT_PROTECTION_DESIRED 1
>  #define DRM_MODE_CONTENT_PROTECTION_ENABLED 2
>  
> +enum extended_colorimetry {
> + EXTENDED_COLORIMETRY_XV_YCC_601 = 0,
> + EXTENDED_COLORIMETRY_XV_YCC_709,
> + EXTENDED_COLORIMETRY_S_YCC_601,
> + EXTENDED_COLORIMETRY_ADOBE_YCC_601,
> + EXTENDED_COLORIMETRY_ADOBE_RGB,
> + EXTENDED_COLORIMETRY_BT2020_RGB,
> + EXTENDED_COLORIMETRY_BT2020_YCC,
> + EXTENDED_COLORIMETRY_BT2020_CYCC,
> +};

This doesn't give any way to distinguish "not set" from BT.601, which
I'm not sure I like.

Is this enum simply built to match the values you're injecting into the
InfoFrame? Would we need a different enum for DisplayPort?

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/cirrus: flip default to 32bpp

[Adam Jackson]

On Fri, 2018-07-06 at 11:12 +0200, Gerd Hoffmann wrote:
> cirrus can handle 1024x768 (and slightly higher) with 24bpp depth.
> cirrus can handle up to 800x600 with 32bpp.

16bpp is maybe a better choice? Nobody's using cirrus because they care
about color fidelity and it'll use less CPU to update. There's
precedent here, mgag200 defaults to 16bpp on sufficiently memory-
impaired devices.

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/10] drm/uapi: Deprecate nonsense kms mode types

[Adam Jackson]

On Tue, 2017-11-14 at 20:32 +0200, Ville Syrjala wrote:

> @@ -253,8 +251,11 @@ struct drm_display_mode {
>*  - DRM_MODE_TYPE_USERDEF: Mode defined via kernel command line
>*
>* Plus a big list of flags which shouldn't be used at all, but are
> -  * still around since these flags are also used in the userspace ABI:
> +  * still around since these flags are also used in the userspace ABI.
> +  * We no longer accept modes with these types though:
>*
> +  *  - DRM_MODE_TYPE_BUILTIN: Meant for hard-coded modes, effectively
> +  *unused.

This should suggest _TYPE_DRIVER, I think. The intent with the xfree86
mode type was that "built-in" modes were known quantities of the
hardware, either because the hardware only had one mode or due to
strapping pins or the like. These should be considered "supplied by the
driver" as with EDID-derived modes.

With or without that, patches 2 3 4 and 8 are:

Reviewed-by: Adam Jackson 

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm] configure: Fix the check

[Adam Jackson]

AC_HEADER_MAJOR only defines MAJOR_IN_SYSMACROS if major() is _not_
defined by  alone. It is, but it warns, and that's ugly.
To fix this, push -Werror into CFLAGS when invoking AC_HEADER_MAJOR so
the warning makes the compilation test fail.

Signed-off-by: Adam Jackson 
---
 configure.ac | 4 
 1 file changed, 4 insertions(+)

diff --git a/configure.ac b/configure.ac
index e5158b7d..43fcf68f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -54,7 +54,11 @@ AC_USE_SYSTEM_EXTENSIONS
 AC_SYS_LARGEFILE
 AC_FUNC_ALLOCA
 
+save_CFLAGS="$CFLAGS"
+export CFLAGS="$CFLAGS -Werror"
 AC_HEADER_MAJOR
+CFLAGS="$save_CFLAGS"
+
 AC_CHECK_HEADERS([sys/sysctl.h sys/select.h])
 
 # Initialize libtool
-- 
2.12.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm] Export drmCompareDevices

[Adam Jackson]

drmCompareBusInfo was almost this already, but it wasn't exported, its
name didn't match its functionality, and while it almost looks like it
was usable for sorting due to memcmp it wouldn't work if you had
multiple bus types. I don't really want to think about defining a
sensible sort order for bus types, so let's at least make it less of a
trap for the caller.

Invert its boolean sense to be 'true if equal', rename it to describe
the type it actually operates on, and export.

Signed-off-by: Adam Jackson 
---
 xf86drm.c | 18 +-
 xf86drm.h |  2 ++
 2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 685cf69d..e584bdff 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3029,32 +3029,32 @@ static int drmParsePciBusInfo(int maj, int min, 
drmPciBusInfoPtr info)
 #endif
 }
 
-static int drmCompareBusInfo(drmDevicePtr a, drmDevicePtr b)
+int drmCompareDevices(drmDevicePtr a, drmDevicePtr b)
 {
 if (a == NULL || b == NULL)
-return -1;
+return 0;
 
 if (a->bustype != b->bustype)
-return -1;
+return 0;
 
 switch (a->bustype) {
 case DRM_BUS_PCI:
-return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo));
+return memcmp(a->businfo.pci, b->businfo.pci, sizeof(drmPciBusInfo)) 
== 0;
 
 case DRM_BUS_USB:
-return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo));
+return memcmp(a->businfo.usb, b->businfo.usb, sizeof(drmUsbBusInfo)) 
== 0;
 
 case DRM_BUS_PLATFORM:
-return memcmp(a->businfo.platform, b->businfo.platform, 
sizeof(drmPlatformBusInfo));
+return memcmp(a->businfo.platform, b->businfo.platform, 
sizeof(drmPlatformBusInfo)) == 0;
 
 case DRM_BUS_HOST1X:
-return memcmp(a->businfo.host1x, b->businfo.host1x, 
sizeof(drmHost1xBusInfo));
+return memcmp(a->businfo.host1x, b->businfo.host1x, 
sizeof(drmHost1xBusInfo)) == 0;
 
 default:
 break;
 }
 
-return -1;
+return 0;
 }
 
 static int drmGetNodeType(const char *name)
@@ -3669,7 +3669,7 @@ static void drmFoldDuplicatedDevices(drmDevicePtr 
local_devices[], int count)
 
 for (i = 0; i < count; i++) {
 for (j = i + 1; j < count; j++) {
-if (drmCompareBusInfo(local_devices[i], local_devices[j]) == 0) {
+if (drmCompareDevices(local_devices[i], local_devices[j])) {
 local_devices[i]->available_nodes |= 
local_devices[j]->available_nodes;
 node_type = log2(local_devices[j]->available_nodes);
 memcpy(local_devices[i]->nodes[node_type],
diff --git a/xf86drm.h b/xf86drm.h
index d75ca8ce..14d2db73 100644
--- a/xf86drm.h
+++ b/xf86drm.h
@@ -851,6 +851,8 @@ extern void drmFreeDevices(drmDevicePtr devices[], int 
count);
 extern int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr *device);
 extern int drmGetDevices2(uint32_t flags, drmDevicePtr devices[], int 
max_devices);
 
+extern int drmCompareDevices(drmDevicePtr a, drmDevicePtr b);
+
 #if defined(__cplusplus)
 }
 #endif
-- 
2.12.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Document code of conduct

[Adam Jackson]

On Tue, 2017-04-18 at 12:10 +0200, Daniel Vetter wrote:
> freedesktop.org has adopted a formal&enforced code of conduct:
> 
> https://www.fooishbar.org/blog/fdo-contributor-covenant/
> https://www.freedesktop.org/wiki/CodeOfConduct/
> 
> Besides formalizing things a bit more I don't think this changes
> anything for us, we've already peer-enforced respectful and
> constructive interactions since a long time. But it's good to document
> things properly.
> 
> v2: Drop confusing note from commit message and clarify the grammer
> (Chris, Alex and others).

Acked-by: Adam Jackson 

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/edid: Make the detailed timing CEA/HDMI mode fixup accept up to 5kHz clock difference

[Adam Jackson]

On Thu, 2015-11-26 at 19:01 +0200, Ville Syrjälä wrote:
> On Mon, Nov 16, 2015 at 09:05:12PM +0200, ville.syrjala at linux.intel.com 
> wrote:
> > From: Ville Syrjälä 
> > 
> > Rather than using drm_match_cea_mode() to see if the EDID detailed
> > timings are supposed to represent one of the CEA/HDMI modes, add a
> > special version of that function that takes in an explicit clock
> > tolerance value (in kHz). When looking at the detailed timings specify
> > the tolerance as 5kHz due to the 10kHz clock resolution limit inherent
> > in detailed timings.
> > 
> > drm_match_cea_mode() uses the normal KHZ2PICOS() matching of clocks,
> > which only allows smaller errors for lower clocks (eg. for 25200 it
> > won't allow any error) and a bigger error for higher clocks (eg. for
> > 297000 it actually matches 296913-297000). So it doesn't really match
> > what we want for the fixup. Using the explicit +-5kHz is much better
> > for this use case.
> > 
> > Not sure if we should change the normal mode matching to also use
> > something else besides KHZ2PICOS() since it allows a different
> > proportion of error depending on the clock. I believe VESA CVT
> > allows a maximum deviation of .5%, so using that for normal mode
> > matching might be a good idea?
> 
> Ping. Any thoughts from anyone?

Reviewed-by: Adam Jackson 

- ajax

[PATCH 1/3] drm/edid: Fix up clock for CEA/HDMI modes specified via detailed timings

[Adam Jackson]

On Thu, 2015-10-08 at 11:43 +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> EDID detailed timings have a resolution of 10kHz for the pixel clock, so
> they can't represent certain CEA/HDMI modes accurately. If we see a mode
> coming in via detailed timings which otherwise matches one of the
> CEA/HDMI modes except the clock is just a bit off, let's assume that the
> intention was for that mode to be one of the CEA/HDMI modes and go ahead
> and fix up the clock to match the CEA/HDMI spec exactly (well, as close
> as we can get with the 1 kHz resolution we use).
> 
> This should help code that's looking for an exact clock match (eg. i915
> audio N/CTS setup).

Looks like a sane set of changes.  Series is:

Reviewed-by: Adam Jackson 

- ajax

[PATCH] drm/mgag200: Reject non-character-cell-aligned mode widths

[Adam Jackson]

Turns out 1366x768 does not in fact work on this hardware.

Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/mgag200/mgag200_mode.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c 
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 6e84df9..ad4b901 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1526,6 +1526,11 @@ static int mga_vga_mode_valid(struct drm_connector 
*connector,
return MODE_BANDWIDTH;
}

+   if ((mode->hdisplay % 8) != 0 || (mode->hsync_start % 8) != 0 ||
+   (mode->hsync_end % 8) != 0 || (mode->htotal % 8) != 0) {
+   return MODE_H_ILLEGAL;
+   }
+
if (mode->crtc_hdisplay > 2048 || mode->crtc_hsync_start > 4096 ||
mode->crtc_hsync_end > 4096 || mode->crtc_htotal > 4096 ||
mode->crtc_vdisplay > 2048 || mode->crtc_vsync_start > 4096 ||
-- 
2.1.0

[PATCH] drm/edid: Add CEA modes before inferred modes

[Adam Jackson]

On Fri, 2015-05-08 at 17:45 +0300, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> Currently we're adding CEA modes after the inferred modes, which 
> means
> we might get multiple modes that are very close to each other, but
> slightly different, which seems a bit silly. That's because duplicate
> mode check that occurs when adding inferred modes would not consider
> CEA modes as potential duplicates. Reverse the order so that CEA
> modes get added before inferred modes, and are thus considered 
> potential
> duplicates.
> 
> Or as ajax put it on irc:
> "< ajax> the point of the "pick a timing formula" heuristic was to
> generate something the sink could _likely_ sink.  if it tells us
> timings it can sink explicitly then second-guessing seems dumb."
> 
> Cc: Adam Jackson 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Adam Jackson 

- ajax

[PATCH 0/3] drm: Basic mode sanity checks

[Adam Jackson]

On Wed, 2014-12-17 at 18:30 +0100, Daniel Vetter wrote:

> All pulled into my drm misc branch.

Ugh, wish I'd caught this earlier.  I'm not entirely thrilled with
requiring non-zero clock, it's not a concept that makes sense on virtual
hardware.

- ajax

[PATCH] drm/vgem: implement virtual GEM

[Adam Jackson]

On Thu, 2014-11-20 at 16:26 -0800, Zach Reizner wrote:
> This patch implements the virtual GEM driver with PRIME sharing which
> allows vgem to import a gem object from other drivers for the purpose
> of mmap-ing them to userspace.

The reason I initially wanted this was to get zero-copy llvmpipe, since
(besides making GLX conformance impossible) the image transfer parts of
drisw are easily the biggest part of the runtime overhead.  Is there a
userspace consumer of this anywhere?

- ajax

[PATCH] allow 32bpp framebuffers for cirrus drm

[Adam Jackson]

On Mon, 2014-10-27 at 16:30 +0100, Gerd Hoffmann wrote:

> How about stop using cirrus and go for 'qemu -vga std' instead?
> 
> Linux kernel 3.14+ comes with a modesetting driver for the qemu standard
> vga (CONFIG_DRM_BOCHS).  Just switch over, and all your cirrus pain is
> gone.
> 
> That is much better than trying use features the real cirrus hardware
> never had, then praying that all qemu versions in the wild are actually
> doing what you want qemu do.

I was going to ask for a pci revision id bump at the same time, so the
guest could know.

I heartily agree, people should stop using cirrus.  And qemu should stop
defaulting to it.  Since neither has happened yet, enhancing the
emulation holds the promise of making the future better...

- ajax

[PATCH] allow 32bpp framebuffers for cirrus drm

[Adam Jackson]

On Fri, 2014-10-24 at 18:31 -0700, Stéphane Marchesin wrote:

> Dave, Adam: are you ok with this patch?

Seems like it doesn't go far enough?  You'd already need an "aware"
guest to have this work, since the chip actually being emulated didn't
have 32bpp.  The pitch check would prevent 1024x768x32 and larger from
working, which makes it sort of a weak win on its own.

Could we read the BAR size from the device instead of hardcoding 4M?
That alone would make 1920x1200x16 work; and then, if that pitch limit
isn't encoded into the register space representation, the aware guest
would have resolutions worth using at least be possible.

- ajax

[PATCH] drm: edid: enable probing and listing of non rb modes

[Adam Jackson]

On Fri, 2013-11-15 at 10:38 +0530, Shirish S wrote:
> The current solution checks for the existing RB mode,
> if available in the edid block returns by adding it,
> but does not populate the connector with the modes
> of same resolution but which are non-rb modes.
> 
> As a result the probing and listing of non-rb modes can't
> be made, in case the rb mode's pixel clock is not
> supported but non-rb mode is supported.

This is... almost okay.

So the EDID 1.4 spec says that for modes in standard descriptors:

a) if there's a match in DMT, use DMT
b) if there's a range descriptor and it says GTF, use GTF
c) if there's a range descriptor and it says CVT, use CVT
d) if there's no range descriptor, use CVT

But case d) is clearly insane if the sink is EDID 1.3, CVT wasn't a
standard when 1.3 was defined, so the sink may not in fact support CVT
timings.  Hence the logic in standard_timing_level().

The other thing the spec says is that if you're using CVT for standard
descriptors that you should use normal blanking instead of reduced.
This is... problematic.  If we see 1920x1200 at 60 in a standard descriptor
we almost certainly _don't_ mean normal blanking, because that won't fit
on single-link DVI.  And if both the source and the sink support reduced
blanking we should probably prefer it, it's marginally less power
consumption on digital links and marginally better picture quality on
analog links.

So probably what we should do instead is:

- if drm_monitor_supports_rb(), add both normal and reduced blanking
timings for either the DMT or CVT path
- extend drm_connector to also have rb-allowed to match interlace and
doublescan
- fix up all the drivers to indicate rb-allowed (except whatever broken
thing it is you're trying to work around here)
- fix drm_mode_validate_flag and callers to filter out rb modes as
appropriate

Also: CVT dates to 2003.  EDID 1.4 was 2006.  Any hardware being made in
2013 that can't generate RB timings is going out of its way to be
broken.  You should really demand better from your hardware.

> 1. adds "rb" suffix to rb modes.

No.  The userspace convention is:

dmt:~% cvt -r 1920 1080
# 1920x1080 59.93 Hz (CVT 2.07M9-R) hsync: 66.59 kHz; pclk: 138.50 MHz
Modeline "1920x1080R"  138.50  1920 1968 2000 2080  1080 1083 1088  +hsync 
-vsync

drm_mode_parse_command_line_for_connector() expects the same thing.
Let's be consistent.

> @@ -1621,19 +1621,20 @@ drm_mode_std(struct drm_connector *connector, struct 
> edid *edid,
>   mode->hdisplay = 1366;
>   mode->hsync_start = mode->hsync_start - 1;
>   mode->hsync_end = mode->hsync_end - 1;
> - return mode;
> + goto done;
>   }
>  
>   /* check whether it can be found in default mode table */
>   if (drm_monitor_supports_rb(edid)) {
>   mode = drm_mode_find_dmt(dev, hsize, vsize, vrefresh_rate,
>true);
> - if (mode)
> - return mode;
> + if (mode) {
> + drm_mode_probed_add(connector, mode);
> + modes++;
> + }
>   }
>   mode = drm_mode_find_dmt(dev, hsize, vsize, vrefresh_rate, false);
> - if (mode)
> - return mode;
> + goto done;
>  
>   /* okay, generate it */
>   switch (timing_level) {

Unconditional "goto done" that skips all the logic for generating modes
not in the dmt list?  This breaks everything _but_ case a) above, so eg.
1600x900 at 60 would no longer be generated.

- ajax

Re: [PATCH 2/2] drm/i915/dp: downstream port capabilities are not present in DPCD 1.0

[Adam Jackson]

On Fri, 2013-09-27 at 14:48 +0300, Jani Nikula wrote:
> We haven't read the downstream port caps for DPCD 1.0, so don't use
> them.
> 
> v2: use defines for DPCD 1.0 downstream port types
> 
> Signed-off-by: Jani Nikula 

I don't know if I've ever seen a DPCD 1.0 device, but the changes make
sense.  For the series:

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/16] drm/mgag200: Reject modes when h-parameters are no multiple of 8

[Adam Jackson]

On Tue, 2013-07-23 at 15:46 +1000, Dave Airlie wrote:
> On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich  wrote:
> > Matrox hardware only supports modes whose horizontal parameters are
> > multiples of 8. This rules out a mode like 1366x768 for example.
> >
> > Signed-off-by: Egbert Eich 
> 
> I'd like to get a second opinion from ajax for where this should be
> dealt with, though this is probably okay.

Patch seems like it's in the right place to me.

- ajax

Re: [PATCH 15/16] drm/mgag200: Reject modes when h-parameters are no multiple of 8

[Adam Jackson]

On Tue, 2013-07-23 at 15:46 +1000, Dave Airlie wrote:
> On Wed, Jul 17, 2013 at 11:07 PM, Egbert Eich  wrote:
> > Matrox hardware only supports modes whose horizontal parameters are
> > multiples of 8. This rules out a mode like 1366x768 for example.
> >
> > Signed-off-by: Egbert Eich 
> 
> I'd like to get a second opinion from ajax for where this should be
> dealt with, though this is probably okay.

Patch seems like it's in the right place to me.

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/radeon: Disable writeback by default on ppc

[Adam Jackson]

On Mon, 2013-06-17 at 11:04 -0400, Alex Deucher wrote:
> On Mon, Jun 17, 2013 at 10:06 AM, Adam Jackson  wrote:
> > At least on an IBM Power 720, this check passes, but several piglit
> > tests will reliably trigger GPU resets due to the ring buffer pointers
> > not being updated.  There's probably a better way to limit this to just
> > affected machines though.
> 
> What radeon chips are you seeing this on?  wb is more or less required
> on r6xx and newer and I'm not sure those generations will even work
> properly without writeback enabled these days.  We force it to always
> be enabled on APUs and NI and newer asics.  With KMS, wb encompasses
> more than just rptr writeback; it covers pretty much everything
> involving the GPU writing any status information to memory.

FirePro 2270, at least.  Booting with no_wb=1, piglit runs to completion
with no GPU resets or IB submission failures.  Booting without no_wb,
the following piglits go from pass to fail, all complaining that the
kernel rejected CS:

   no-wb wb
   (info)(info)
All7598/9587 7403/9553 
glean  362/385   361/385   
pointAtten pass  fail  
texture_srgb   pass  fail  
shaders477/533   441/533   
glsl-algebraic-add-add-1   pass  fail  
glsl-algebraic-add-add-2   pass  fail  
glsl-algebraic-add-add-3   pass  fail  
glsl-algebraic-sub-zero-3  pass  fail  
glsl-algebraic-sub-zero-4  pass  fail  
glsl-complex-subscript pass  fail  
glsl-copy-propagation-if-1 pass  fail  
glsl-copy-propagation-if-2 pass  fail  
glsl-copy-propagation-if-3 pass  fail  
glsl-copy-propagation-vector-indexing  pass  fail  
glsl-fs-atan-2 pass  fail  
glsl-fs-dot-vec2-2 pass  fail  
glsl-fs-log2   pass  fail  
glsl-fs-main-returnpass  fail  
glsl-fs-max-3  pass  fail  
glsl-fs-min-2  pass  fail  
glsl-fs-min-3  pass  fail  
glsl-fs-statevar-call  pass  fail  
glsl-fs-struct-equal   pass  fail  
glsl-function-chain16  pass  fail  
glsl-implicit-conversion-02pass  fail  
glsl-inout-struct-01   pass  fail  
glsl-inout-struct-02   pass  fail  
glsl-link-varying-TexCoord pass  fail  
glsl-link-varyings-2   pass  fail  
glsl-uniform-initializer-4 pass  fail  
glsl-uniform-initializer-6 pass  fail  
glsl-uniform-initializer-7 pass  fail  
glsl-vs-abs-neg-with-intermediate  pass  fail  
glsl-vs-clamp-1pass  fail  
glsl-vs-deadcode-1 pass  fail  
glsl-vs-deadcode-2 pass  fail  
glsl-vs-f2bpass  fail  
glsl-vs-position-outvalpass  fail  
link-uniform-array-sizepass  fail  
loopfunc   pass  fail  
spec   5921/7801 5763/7767 
!OpenGL 1.1118/229   101/219   
depthstencil-default_fb-clear  pass  fail  
getteximage-simple pass  fail  
texwrap formats27/50 12/40 
GL_LUMINANCE12 pass  fail  
GL_LUMINANCE16 pass  fail  
GL_LUMINANCE4  pass  fail  
GL_LUMINANCE4_ALPHA4   

[PATCH] drm/radeon: Disable writeback by default on ppc

[Adam Jackson]

At least on an IBM Power 720, this check passes, but several piglit
tests will reliably trigger GPU resets due to the ring buffer pointers
not being updated.  There's probably a better way to limit this to just
affected machines though.

Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/radeon/r600_cp.c   | 7 +++
 drivers/gpu/drm/radeon/radeon_cp.c | 7 +++
 drivers/gpu/drm/radeon/radeon_device.c | 4 ++--
 drivers/gpu/drm/radeon/radeon_drv.c| 2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600_cp.c b/drivers/gpu/drm/radeon/r600_cp.c
index 1c51c08..ef28532 100644
--- a/drivers/gpu/drm/radeon/r600_cp.c
+++ b/drivers/gpu/drm/radeon/r600_cp.c
@@ -552,6 +552,13 @@ static void r600_test_writeback(drm_radeon_private_t 
*dev_priv)
dev_priv->writeback_works = 0;
DRM_INFO("writeback test failed\n");
}
+#if defined(__ppc__) || defined(__ppc64__)
+   /* the test might succeed on ppc, but it's usually not reliable */
+   if (radeon_no_wb == -1) {
+   radeon_no_wb = 1;
+   DRM_INFO("not trusting writeback test due to arch quirk\n");
+   }
+#endif
if (radeon_no_wb == 1) {
dev_priv->writeback_works = 0;
DRM_INFO("writeback forced off\n");
diff --git a/drivers/gpu/drm/radeon/radeon_cp.c 
b/drivers/gpu/drm/radeon/radeon_cp.c
index efc4f64..a967b33 100644
--- a/drivers/gpu/drm/radeon/radeon_cp.c
+++ b/drivers/gpu/drm/radeon/radeon_cp.c
@@ -892,6 +892,13 @@ static void radeon_test_writeback(drm_radeon_private_t * 
dev_priv)
dev_priv->writeback_works = 0;
DRM_INFO("writeback test failed\n");
}
+#if defined(__ppc__) || defined(__ppc64__)
+   /* the test might succeed on ppc, but it's usually not reliable */
+   if (radeon_no_wb == -1) {
+   radeon_no_wb = 1;
+   DRM_INFO("not trusting writeback test due to arch quirk\n");
+   }
+#endif
if (radeon_no_wb == 1) {
dev_priv->writeback_works = 0;
DRM_INFO("writeback forced off\n");
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index 1899738..524046e 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -322,8 +322,8 @@ int radeon_wb_init(struct radeon_device *rdev)
/* disable event_write fences */
rdev->wb.use_event = false;
/* disabled via module param */
-   if (radeon_no_wb == 1) {
-   rdev->wb.enabled = false;
+   if (radeon_no_wb != -1) {
+   rdev->wb.enabled = !!radeon_no_wb;
} else {
if (rdev->flags & RADEON_IS_AGP) {
/* often unreliable on AGP */
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 094e7e5..04809d4 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -146,7 +146,7 @@ static inline void radeon_register_atpx_handler(void) {}
 static inline void radeon_unregister_atpx_handler(void) {}
 #endif

-int radeon_no_wb;
+int radeon_no_wb = -1;
 int radeon_modeset = -1;
 int radeon_dynclks = -1;
 int radeon_r4xx_atom = 0;
-- 
1.8.2.1

Re: [PATCH] drm/radeon: Disable writeback by default on ppc

[Adam Jackson]

On Mon, 2013-06-17 at 11:04 -0400, Alex Deucher wrote:
> On Mon, Jun 17, 2013 at 10:06 AM, Adam Jackson  wrote:
> > At least on an IBM Power 720, this check passes, but several piglit
> > tests will reliably trigger GPU resets due to the ring buffer pointers
> > not being updated.  There's probably a better way to limit this to just
> > affected machines though.
> 
> What radeon chips are you seeing this on?  wb is more or less required
> on r6xx and newer and I'm not sure those generations will even work
> properly without writeback enabled these days.  We force it to always
> be enabled on APUs and NI and newer asics.  With KMS, wb encompasses
> more than just rptr writeback; it covers pretty much everything
> involving the GPU writing any status information to memory.

FirePro 2270, at least.  Booting with no_wb=1, piglit runs to completion
with no GPU resets or IB submission failures.  Booting without no_wb,
the following piglits go from pass to fail, all complaining that the
kernel rejected CS:

   no-wb wb
   (info)(info)
All7598/9587 7403/9553 
glean  362/385   361/385   
pointAtten pass  fail  
texture_srgb   pass  fail  
shaders477/533   441/533   
glsl-algebraic-add-add-1   pass  fail  
glsl-algebraic-add-add-2   pass  fail  
glsl-algebraic-add-add-3   pass  fail  
glsl-algebraic-sub-zero-3  pass  fail  
glsl-algebraic-sub-zero-4  pass  fail  
glsl-complex-subscript pass  fail  
glsl-copy-propagation-if-1 pass  fail  
glsl-copy-propagation-if-2 pass  fail  
glsl-copy-propagation-if-3 pass  fail  
glsl-copy-propagation-vector-indexing  pass  fail  
glsl-fs-atan-2 pass  fail  
glsl-fs-dot-vec2-2 pass  fail  
glsl-fs-log2   pass  fail  
glsl-fs-main-returnpass  fail  
glsl-fs-max-3  pass  fail  
glsl-fs-min-2  pass  fail  
glsl-fs-min-3  pass  fail  
glsl-fs-statevar-call  pass  fail  
glsl-fs-struct-equal   pass  fail  
glsl-function-chain16  pass  fail  
glsl-implicit-conversion-02pass  fail  
glsl-inout-struct-01   pass  fail  
glsl-inout-struct-02   pass  fail  
glsl-link-varying-TexCoord pass  fail  
glsl-link-varyings-2   pass  fail  
glsl-uniform-initializer-4 pass  fail  
glsl-uniform-initializer-6 pass  fail  
glsl-uniform-initializer-7 pass  fail  
glsl-vs-abs-neg-with-intermediate  pass  fail  
glsl-vs-clamp-1pass  fail  
glsl-vs-deadcode-1 pass  fail  
glsl-vs-deadcode-2 pass  fail  
glsl-vs-f2bpass  fail  
glsl-vs-position-outvalpass  fail  
link-uniform-array-sizepass  fail  
loopfunc   pass  fail  
spec   5921/7801 5763/7767 
!OpenGL 1.1118/229   101/219   
depthstencil-default_fb-clear  pass  fail  
getteximage-simple pass  fail  
texwrap formats27/50 12/40 
GL_LUMINANCE12 pass  fail  
GL_LUMINANCE16 pass  fail  
GL_LUMINANCE4  pass  fail  
GL_LUMINANCE4_ALPHA4   

[PATCH] drm/radeon: Disable writeback by default on ppc

[Adam Jackson]

At least on an IBM Power 720, this check passes, but several piglit
tests will reliably trigger GPU resets due to the ring buffer pointers
not being updated.  There's probably a better way to limit this to just
affected machines though.

Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/radeon/r600_cp.c   | 7 +++
 drivers/gpu/drm/radeon/radeon_cp.c | 7 +++
 drivers/gpu/drm/radeon/radeon_device.c | 4 ++--
 drivers/gpu/drm/radeon/radeon_drv.c| 2 +-
 4 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/radeon/r600_cp.c b/drivers/gpu/drm/radeon/r600_cp.c
index 1c51c08..ef28532 100644
--- a/drivers/gpu/drm/radeon/r600_cp.c
+++ b/drivers/gpu/drm/radeon/r600_cp.c
@@ -552,6 +552,13 @@ static void r600_test_writeback(drm_radeon_private_t 
*dev_priv)
dev_priv->writeback_works = 0;
DRM_INFO("writeback test failed\n");
}
+#if defined(__ppc__) || defined(__ppc64__)
+   /* the test might succeed on ppc, but it's usually not reliable */
+   if (radeon_no_wb == -1) {
+   radeon_no_wb = 1;
+   DRM_INFO("not trusting writeback test due to arch quirk\n");
+   }
+#endif
if (radeon_no_wb == 1) {
dev_priv->writeback_works = 0;
DRM_INFO("writeback forced off\n");
diff --git a/drivers/gpu/drm/radeon/radeon_cp.c 
b/drivers/gpu/drm/radeon/radeon_cp.c
index efc4f64..a967b33 100644
--- a/drivers/gpu/drm/radeon/radeon_cp.c
+++ b/drivers/gpu/drm/radeon/radeon_cp.c
@@ -892,6 +892,13 @@ static void radeon_test_writeback(drm_radeon_private_t * 
dev_priv)
dev_priv->writeback_works = 0;
DRM_INFO("writeback test failed\n");
}
+#if defined(__ppc__) || defined(__ppc64__)
+   /* the test might succeed on ppc, but it's usually not reliable */
+   if (radeon_no_wb == -1) {
+   radeon_no_wb = 1;
+   DRM_INFO("not trusting writeback test due to arch quirk\n");
+   }
+#endif
if (radeon_no_wb == 1) {
dev_priv->writeback_works = 0;
DRM_INFO("writeback forced off\n");
diff --git a/drivers/gpu/drm/radeon/radeon_device.c 
b/drivers/gpu/drm/radeon/radeon_device.c
index 1899738..524046e 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -322,8 +322,8 @@ int radeon_wb_init(struct radeon_device *rdev)
/* disable event_write fences */
rdev->wb.use_event = false;
/* disabled via module param */
-   if (radeon_no_wb == 1) {
-   rdev->wb.enabled = false;
+   if (radeon_no_wb != -1) {
+   rdev->wb.enabled = !!radeon_no_wb;
} else {
if (rdev->flags & RADEON_IS_AGP) {
/* often unreliable on AGP */
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c 
b/drivers/gpu/drm/radeon/radeon_drv.c
index 094e7e5..04809d4 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -146,7 +146,7 @@ static inline void radeon_register_atpx_handler(void) {}
 static inline void radeon_unregister_atpx_handler(void) {}
 #endif
 
-int radeon_no_wb;
+int radeon_no_wb = -1;
 int radeon_modeset = -1;
 int radeon_dynclks = -1;
 int radeon_r4xx_atom = 0;
-- 
1.8.2.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/edid: Don't print messages regarding stereo or csync by default

[Adam Jackson]

On Thu, 2013-06-13 at 21:01 +0200, Egbert Eich wrote:
> drm_mode_detailed() is called quite often, therefore when a monitor
> that has a detailed timing mode marked DRM_EDID_PT_STEREO or requiring
> composite sync, warning messages will clutter up the kernel log.
> Like we already do for incorrect hsync/vsync pluse widths, print these
> messages only when KMS debugging is enabled.

Oof, yes, a thousand times yes, how were we still doing this.

Reviewed-by: Adam Jackson 

- ajax
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20130613/f3920b1e/attachment.pgp>

Re: [PATCH] drm/edid: Don't print messages regarding stereo or csync by default

[Adam Jackson]

On Thu, 2013-06-13 at 21:01 +0200, Egbert Eich wrote:
> drm_mode_detailed() is called quite often, therefore when a monitor
> that has a detailed timing mode marked DRM_EDID_PT_STEREO or requiring
> composite sync, warning messages will clutter up the kernel log.
> Like we already do for incorrect hsync/vsync pluse widths, print these
> messages only when KMS debugging is enabled.

Oof, yes, a thousand times yes, how were we still doing this.

Reviewed-by: Adam Jackson 

- ajax


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: don't add inferred modes for monitors that don't support them

[Adam Jackson]

On Fri, 2013-02-15 at 13:36 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni 
> 
> If bit 0 of the features byte (0x18) is set to 0, then, according to
> the EDID spec, "the display is non-continuous frequency (multi-mode)
> and is only specified to accept the video timing formats that are
> listed in Base EDID and certain Extension Blocks".
> 
> For more information, please see the EDID spec, check the notes of the
> table that explains the "Feature Support" byte (18h) and also the
> notes on the tables of the section that explains "Display Range Limits
> & Additional Timing Description Definition (tag #FDh)".

Seems sane.

Reviewed-by: Adam Jackson 

- ajax

Re: [PATCH] drm: don't add inferred modes for monitors that don't support them

[Adam Jackson]

On Fri, 2013-02-15 at 13:36 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni 
> 
> If bit 0 of the features byte (0x18) is set to 0, then, according to
> the EDID spec, "the display is non-continuous frequency (multi-mode)
> and is only specified to accept the video timing formats that are
> listed in Base EDID and certain Extension Blocks".
> 
> For more information, please see the EDID spec, check the notes of the
> table that explains the "Feature Support" byte (18h) and also the
> notes on the tables of the section that explains "Display Range Limits
> & Additional Timing Description Definition (tag #FDh)".

Seems sane.

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/i915: move panel connectors to the front

[Adam Jackson]

On 10/27/12 9:52 AM, Daniel Vetter wrote:
> This essentially reverts
>
> commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> Author: Adam Jackson 
> Date:   Fri Jul 16 14:46:29 2010 -0400
>
>  drm/i915: Initialize LVDS and eDP outputs before anything else
>
> simply because it doesn't scale: It misses SDVO and DVO panels,
> and now with DDI encoders on haswell this is becoming unmanageable.
>
> Instead we simply sort the connector list after everything is
> set up.
>
> Signed-off-by: Daniel Vetter 

Reviewed-by: Adam Jackson 

- ajax

[PATCH 1/2] drm: add helper to sort panels to the head of the connector list

[Adam Jackson]

On 10/27/12 9:52 AM, Daniel Vetter wrote:
> Userspace seems to like this, see
>
> commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
> Author: Adam Jackson 
> Date:   Fri Jul 16 14:46:29 2010 -0400
>
>  drm/i915: Initialize LVDS and eDP outputs before anything else
>
>  This makes them sort to the front in X, which makes them likely to be
>  the primary outputs if you haven't specified a preference in your DE,
>  which is likely to be what you want.
>
>  Signed-off-by: Adam Jackson 
>  Signed-off-by: Eric Anholt 
>
> Sorting the connector list after the fact is much easier than trying
> to be clever with the init sequence.

Entirely reasonable.

Reviewed-by: Adam Jackson 

- ajax

Re: [PATCH 2/2] drm/i915: move panel connectors to the front

[Adam Jackson]

On 10/27/12 9:52 AM, Daniel Vetter wrote:

This essentially reverts

commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
Author: Adam Jackson 
Date:   Fri Jul 16 14:46:29 2010 -0400

 drm/i915: Initialize LVDS and eDP outputs before anything else

simply because it doesn't scale: It misses SDVO and DVO panels,
and now with DDI encoders on haswell this is becoming unmanageable.

Instead we simply sort the connector list after everything is
set up.

Signed-off-by: Daniel Vetter 


Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: add helper to sort panels to the head of the connector list

[Adam Jackson]

On 10/27/12 9:52 AM, Daniel Vetter wrote:

Userspace seems to like this, see

commit cb0953d734348e8862d6d7edc666cfb3bf6d8fae
Author: Adam Jackson 
Date:   Fri Jul 16 14:46:29 2010 -0400

 drm/i915: Initialize LVDS and eDP outputs before anything else

 This makes them sort to the front in X, which makes them likely to be
 the primary outputs if you haven't specified a preference in your DE,
 which is likely to be what you want.

 Signed-off-by: Adam Jackson 
 Signed-off-by: Eric Anholt 

Sorting the connector list after the fact is much easier than trying
to be clever with the init sequence.


Entirely reasonable.

Reviewed-by: Adam Jackson 

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

radeon: RFC speed cap detection on ppc64

[Adam Jackson]

On 10/19/12 1:43 PM, Lucas Kannebley Tavares wrote:
> The radeon driver does speed cap detection on the root PCI device for
> the maximum speed with which the adapter can communicate. On ppc64
> systems, however, the root device belongs to the Hypervisor, so the
> current code would case a null pointer dereference.
>
> I propose to look for the outmost bus with a parent node and get speed
> caps from it, though I suppose the cleaner way would be to inspect all
> devices along the way and choose the smallest speed cap.

That (walking all parent nodes) is probably the safest thing to do.  I'm 
not sure whether it's optimal.  It would likely depend on whether you 
can meaningfully have a bridge that's faster on the downstream side than 
on the upstream.

- ajax

Re: radeon: RFC speed cap detection on ppc64

[Adam Jackson]

On 10/19/12 1:43 PM, Lucas Kannebley Tavares wrote:

The radeon driver does speed cap detection on the root PCI device for
the maximum speed with which the adapter can communicate. On ppc64
systems, however, the root device belongs to the Hypervisor, so the
current code would case a null pointer dereference.

I propose to look for the outmost bus with a parent node and get speed
caps from it, though I suppose the cleaner way would be to inspect all
devices along the way and choose the smallest speed cap.


That (walking all parent nodes) is probably the safest thing to do.  I'm 
not sure whether it's optimal.  It would likely depend on whether you 
can meaningfully have a bridge that's faster on the downstream side than 
on the upstream.


- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 0/3] Enhanced EDID quirk functionality

[Adam Jackson]

On 10/8/12 7:22 PM, Ian Pilcher wrote:
> On 09/28/2012 02:03 AM, Ian Pilcher wrote:
>> As promised, this patch series does only the following:
>>
>> * Add user-defined EDID quirks -- via sysfs or a module parameter
>> * Add an EDID quirk flag to disable all HDMI functionality
>>(InfoFrames)
>> * Add a quirk to disable HDMI InfoFrames for the LG L246WP
>>
>
> ACK?  NACK?  I'm on crack?
>
> (I'm starting to think that this is some sort of hazing ritual.)

Sorry, I'm just not paying close attention to EDID stuff at the moment.

The series as is is clearly better than what we've got.  I still think 
there's something fundamentally amiss with a display that doesn't want 
to accept InfoFrames, but without having it in hand to mess with - and 
without wanting to play telephone to figure it out - I'm fine with this.

Reviewed-by: Adam Jackson 

- ajax

Re: [PATCH v5 0/3] Enhanced EDID quirk functionality

[Adam Jackson]

On 10/8/12 7:22 PM, Ian Pilcher wrote:

On 09/28/2012 02:03 AM, Ian Pilcher wrote:

As promised, this patch series does only the following:

* Add user-defined EDID quirks -- via sysfs or a module parameter
* Add an EDID quirk flag to disable all HDMI functionality
   (InfoFrames)
* Add a quirk to disable HDMI InfoFrames for the LG L246WP



ACK?  NACK?  I'm on crack?

(I'm starting to think that this is some sort of hazing ritual.)


Sorry, I'm just not paying close attention to EDID stuff at the moment.

The series as is is clearly better than what we've got.  I still think 
there's something fundamentally amiss with a display that doesn't want 
to accept InfoFrames, but without having it in hand to mess with - and 
without wanting to play telephone to figure it out - I'm fine with this.


Reviewed-by: Adam Jackson 

- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

-next trees

[Adam Jackson]

On Tue, 2012-09-25 at 13:47 -0500, Ian Pilcher wrote:

> In the meantime, I would like to move the ball forward on this issue.
> As I see it, there 3 issues that have become conflated:
> 
> 1.  The display (LG L246WP) is confused by *any* InfoFrames.

If this is still the case after applying:

commit adf00b26d18e1b3570451296e03bcb20e4798cdd
Author: Paulo Zanoni 
Date:   Tue Sep 25 13:23:34 2012 -0300

drm/i915: make sure we write all the DIP data bytes

Then I suspect I'm compelled to agree that we need a quirk to forcibly
disable InfoFrames entirely.  I don't like to be difficult about this,
but the HDMI spec is quite clear that sinks _must_ accept InfoFrames
defined in either the CEA or HDMI specs, so if we're seeing that class
of failure I tend to strongly suspect our drivers first.

> 3.  drm_detect_monitor_audio is returning true for the LG L246WP, which
>  definitely doesn't have any audio capabilities.  This may be a bug
>  in the display's EDID, or it may be a parsing error.

The display is definitely a filthy liar then:

  Audio data block
Linear PCM, max channels 1
Supported sample rates (kHz): 48 44.1 32
Supported sample sizes (bits): 24 20 16

Hooray for hardware.  Not sure what the logic should be for whether to
send HDMI audio or not, I'll re-read the appropriate scrolls.

- ajax
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: 

Re: -next trees

[Adam Jackson]

On Tue, 2012-09-25 at 13:47 -0500, Ian Pilcher wrote:

> In the meantime, I would like to move the ball forward on this issue.
> As I see it, there 3 issues that have become conflated:
> 
> 1.  The display (LG L246WP) is confused by *any* InfoFrames.

If this is still the case after applying:

commit adf00b26d18e1b3570451296e03bcb20e4798cdd
Author: Paulo Zanoni 
Date:   Tue Sep 25 13:23:34 2012 -0300

drm/i915: make sure we write all the DIP data bytes

Then I suspect I'm compelled to agree that we need a quirk to forcibly
disable InfoFrames entirely.  I don't like to be difficult about this,
but the HDMI spec is quite clear that sinks _must_ accept InfoFrames
defined in either the CEA or HDMI specs, so if we're seeing that class
of failure I tend to strongly suspect our drivers first.

> 3.  drm_detect_monitor_audio is returning true for the LG L246WP, which
>  definitely doesn't have any audio capabilities.  This may be a bug
>  in the display's EDID, or it may be a parsing error.

The display is definitely a filthy liar then:

  Audio data block
Linear PCM, max channels 1
Supported sample rates (kHz): 48 44.1 32
Supported sample sizes (bits): 24 20 16

Hooray for hardware.  Not sure what the logic should be for whether to
send HDMI audio or not, I'll re-read the appropriate scrolls.

- ajax


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

-next trees

[Adam Jackson]

On Sat, 2012-09-22 at 23:34 -0500, Ian Pilcher wrote:
> On 09/12/2012 08:44 PM, Dave Airlie wrote:
> > Ajax, I think Ian still has an open request on how to proceed with the
> > infoframes quirk nightmare.
> 
> Indeed.

I'm sorry, I thought I was clearer.  Daniel posted a patch to fix the
Intel driver for this:

http://lists.freedesktop.org/archives/intel-gfx/2012-August/020046.html

Which I didn't entirely ack, but which is essentially right.  That's
what we should do; and having done so, if I understand things correctly,
there's no need for any quirks here at all.

Is there something I'm missing?

But also:

> > * Leave both flags, because both are accurate.  The display is confused
> >   by any InfoFrames (audio or AVI), and it is reporting non-existent
> >   audio capabilities.  The fact that the combination of the two flags
> >   makes the display work with Intel GPUs is a happy/sad/neutral
> >   accident, and the driver's behavior is still considered broken.

I haven't actually seen the EDID block for this display, I don't
believe, so I'm not sure whether the "non-existent" part of this is even
accurate, or whether we're just parsing things incorrectly.  There's a
reason I keep a standalone parser around.

- ajax
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: 

Re: -next trees

[Adam Jackson]

On Sat, 2012-09-22 at 23:34 -0500, Ian Pilcher wrote:
> On 09/12/2012 08:44 PM, Dave Airlie wrote:
> > Ajax, I think Ian still has an open request on how to proceed with the
> > infoframes quirk nightmare.
> 
> Indeed.

I'm sorry, I thought I was clearer.  Daniel posted a patch to fix the
Intel driver for this:

http://lists.freedesktop.org/archives/intel-gfx/2012-August/020046.html

Which I didn't entirely ack, but which is essentially right.  That's
what we should do; and having done so, if I understand things correctly,
there's no need for any quirks here at all.

Is there something I'm missing?

But also:

> > * Leave both flags, because both are accurate.  The display is confused
> >   by any InfoFrames (audio or AVI), and it is reporting non-existent
> >   audio capabilities.  The fact that the combination of the two flags
> >   makes the display work with Intel GPUs is a happy/sad/neutral
> >   accident, and the driver's behavior is still considered broken.

I haven't actually seen the EDID block for this display, I don't
believe, so I'm not sure whether the "non-existent" part of this is even
accurate, or whether we're just parsing things incorrectly.  There's a
reason I keep a standalone parser around.

- ajax


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm/dp: Make sink count DP 1.2 aware

[Adam Jackson]

Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/i915/intel_dp.c |9 -
 include/drm/drm_dp_helper.h |3 ++-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 813b771..00f99e5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2106,13 +2106,12 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
/* If we're HPD-aware, SINK_COUNT changes dynamically */
hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD);
if (hpd) {
-   uint8_t sink_count;
+   uint8_t reg;
if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-   &sink_count, 1))
+   ®, 1))
return connector_status_unknown;
-   sink_count &= DP_SINK_COUNT_MASK;
-   return sink_count ? connector_status_connected
- : connector_status_disconnected;
+   return DP_GET_SINK_COUNT(reg) ? connector_status_connected
+ : connector_status_disconnected;
}

/* If no HPD, poke DDC gently */
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 38ffcb4..fe06148 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -221,7 +221,8 @@
 # define DP_PSR_FRAME_CAPTURE  (1 << 3)

 #define DP_SINK_COUNT  0x200
-# define DP_SINK_COUNT_MASK(31 << 0)
+/* prior to 1.2 bit 7 was reserved mbz */
+# define DP_GET_SINK_COUNT(x)  x) & 0x80) >> 1) | ((x) & 0x3f))
 # define DP_SINK_CP_READY  (1 << 6)

 #define DP_DEVICE_SERVICE_IRQ_VECTOR   0x201
-- 
1.7.7.6

[PATCH 1/2] drm/dp: Document DP spec versions for various DPCD registers

[Adam Jackson]

Note with a comment anything newer than DP 1.1a.

Obviously this needs some work still...

Signed-off-by: Adam Jackson 
---
 include/drm/drm_dp_helper.h |   52 ++
 1 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index f9888c3..38ffcb4 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -26,7 +26,19 @@
 #include 
 #include 

-/* From the VESA DisplayPort spec */
+/*
+ * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
+ * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
+ * 1.0 devices basically don't exist in the wild.
+ *
+ * Abbreviations, in chronological order:
+ *
+ * eDP: Embedded DisplayPort version 1
+ * DPI: DisplayPort Interoperability Guideline v1.1a
+ * 1.2: DisplayPort 1.2
+ *
+ * 1.2 formally includes both eDP and DPI definitions.
+ */

 #define AUX_NATIVE_WRITE   0x8
 #define AUX_NATIVE_READ0x9
@@ -53,7 +65,7 @@

 #define DP_MAX_LANE_COUNT   0x002
 # define DP_MAX_LANE_COUNT_MASK0x1f
-# define DP_TPS3_SUPPORTED (1 << 6)
+# define DP_TPS3_SUPPORTED (1 << 6) /* 1.2 */
 # define DP_ENHANCED_FRAME_CAP (1 << 7)

 #define DP_MAX_DOWNSPREAD   0x003
@@ -69,16 +81,16 @@
 /* 10b = TMDS or HDMI */
 /* 11b = Other */
 # define DP_FORMAT_CONVERSION   (1 << 3)
-# define DP_DETAILED_CAP_INFO_AVAILABLE(1 << 4)
+# define DP_DETAILED_CAP_INFO_AVAILABLE(1 << 4) /* DPI */

 #define DP_MAIN_LINK_CHANNEL_CODING 0x006

 #define DP_DOWN_STREAM_PORT_COUNT  0x007
 # define DP_PORT_COUNT_MASK0x0f
-# define DP_MSA_TIMING_PAR_IGNORED (1 << 6)
+# define DP_MSA_TIMING_PAR_IGNORED (1 << 6) /* eDP */
 # define DP_OUI_SUPPORT(1 << 7)

-#define DP_I2C_SPEED_CAP   0x00c
+#define DP_I2C_SPEED_CAP   0x00c/* DPI */
 # define DP_I2C_SPEED_1K   0x01
 # define DP_I2C_SPEED_5K   0x02
 # define DP_I2C_SPEED_10K  0x04
@@ -86,16 +98,16 @@
 # define DP_I2C_SPEED_400K 0x10
 # define DP_I2C_SPEED_1M   0x20

-#define DP_EDP_CONFIGURATION_CAP0x00d
-#define DP_TRAINING_AUX_RD_INTERVAL 0x00e
+#define DP_EDP_CONFIGURATION_CAP0x00d   /* XXX 1.2? */
+#define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */

 /* Multiple stream transport */
-#define DP_MSTM_CAP0x021
+#define DP_MSTM_CAP0x021   /* 1.2 */
 # define DP_MST_CAP(1 << 0)

-#define DP_PSR_SUPPORT  0x070
+#define DP_PSR_SUPPORT  0x070   /* XXX 1.2? */
 # define DP_PSR_IS_SUPPORTED1
-#define DP_PSR_CAPS 0x071
+#define DP_PSR_CAPS 0x071   /* XXX 1.2? */
 # define DP_PSR_NO_TRAIN_ON_EXIT1
 # define DP_PSR_SETUP_TIME_330  (0 << 1)
 # define DP_PSR_SETUP_TIME_275  (1 << 1)
@@ -136,7 +148,7 @@
 #defineDP_LINK_BW_SET  0x100
 # define DP_LINK_BW_1_62   0x06
 # define DP_LINK_BW_2_70x0a
-# define DP_LINK_BW_5_40x14
+# define DP_LINK_BW_5_40x14/* 1.2 */

 #define DP_LANE_COUNT_SET  0x101
 # define DP_LANE_COUNT_MASK0x0f
@@ -146,7 +158,7 @@
 # define DP_TRAINING_PATTERN_DISABLE   0
 # define DP_TRAINING_PATTERN_1 1
 # define DP_TRAINING_PATTERN_2 2
-# define DP_TRAINING_PATTERN_3 3
+# define DP_TRAINING_PATTERN_3 3   /* 1.2 */
 # define DP_TRAINING_PATTERN_MASK  0x3

 # define DP_LINK_QUAL_PATTERN_DISABLE  (0 << 2)
@@ -187,22 +199,22 @@

 #define DP_DOWNSPREAD_CTRL 0x107
 # define DP_SPREAD_AMP_0_5 (1 << 4)
-# define DP_MSA_TIMING_PAR_IGNORE_EN   (1 << 7)
+# define DP_MSA_TIMING_PAR_IGNORE_EN   (1 << 7) /* eDP */

 #define DP_MAIN_LINK_CHANNEL_CODING_SET0x108
 # define DP_SET_ANSI_8B10B (1 << 0)

-#define DP_I2C_SPEED_CONTROL_STATUS0x109
+#define DP_I2C_SPEED_CONTROL_STATUS0x109   /* DPI */
 /* bitmask as for DP_I2C_SPEED_CAP */

-#define DP_EDP_CONFIGURATION_SET0x10a
+#define DP_EDP_CONFIGURATION_SET0x10a   /* XXX 1.2? */

-#define DP_MSTM_CTRL   0x111
+#define DP_MSTM_CTRL   0x111   /* 1.2 */
 # define DP_MST_EN (1 << 0)
 # define DP_UP_REQ_EN  (1 << 1)
 # define DP_UPSTREAM_IS_SRC  

[PATCH 2/2] drm/dp: Make sink count DP 1.2 aware

[Adam Jackson]

Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/i915/intel_dp.c |9 -
 include/drm/drm_dp_helper.h |3 ++-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 813b771..00f99e5 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2106,13 +2106,12 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
/* If we're HPD-aware, SINK_COUNT changes dynamically */
hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD);
if (hpd) {
-   uint8_t sink_count;
+   uint8_t reg;
if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
-   &sink_count, 1))
+   ®, 1))
return connector_status_unknown;
-   sink_count &= DP_SINK_COUNT_MASK;
-   return sink_count ? connector_status_connected
- : connector_status_disconnected;
+   return DP_GET_SINK_COUNT(reg) ? connector_status_connected
+ : connector_status_disconnected;
}
 
/* If no HPD, poke DDC gently */
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 38ffcb4..fe06148 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -221,7 +221,8 @@
 # define DP_PSR_FRAME_CAPTURE  (1 << 3)
 
 #define DP_SINK_COUNT  0x200
-# define DP_SINK_COUNT_MASK(31 << 0)
+/* prior to 1.2 bit 7 was reserved mbz */
+# define DP_GET_SINK_COUNT(x)  x) & 0x80) >> 1) | ((x) & 0x3f))
 # define DP_SINK_CP_READY  (1 << 6)
 
 #define DP_DEVICE_SERVICE_IRQ_VECTOR   0x201
-- 
1.7.7.6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm/dp: Document DP spec versions for various DPCD registers

[Adam Jackson]

Note with a comment anything newer than DP 1.1a.

Obviously this needs some work still...

Signed-off-by: Adam Jackson 
---
 include/drm/drm_dp_helper.h |   52 ++
 1 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index f9888c3..38ffcb4 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -26,7 +26,19 @@
 #include 
 #include 
 
-/* From the VESA DisplayPort spec */
+/*
+ * Unless otherwise noted, all values are from the DP 1.1a spec.  Note that
+ * DP and DPCD versions are independent.  Differences from 1.0 are not noted,
+ * 1.0 devices basically don't exist in the wild.
+ *
+ * Abbreviations, in chronological order:
+ *
+ * eDP: Embedded DisplayPort version 1
+ * DPI: DisplayPort Interoperability Guideline v1.1a
+ * 1.2: DisplayPort 1.2
+ *
+ * 1.2 formally includes both eDP and DPI definitions.
+ */
 
 #define AUX_NATIVE_WRITE   0x8
 #define AUX_NATIVE_READ0x9
@@ -53,7 +65,7 @@
 
 #define DP_MAX_LANE_COUNT   0x002
 # define DP_MAX_LANE_COUNT_MASK0x1f
-# define DP_TPS3_SUPPORTED (1 << 6)
+# define DP_TPS3_SUPPORTED (1 << 6) /* 1.2 */
 # define DP_ENHANCED_FRAME_CAP (1 << 7)
 
 #define DP_MAX_DOWNSPREAD   0x003
@@ -69,16 +81,16 @@
 /* 10b = TMDS or HDMI */
 /* 11b = Other */
 # define DP_FORMAT_CONVERSION   (1 << 3)
-# define DP_DETAILED_CAP_INFO_AVAILABLE(1 << 4)
+# define DP_DETAILED_CAP_INFO_AVAILABLE(1 << 4) /* DPI */
 
 #define DP_MAIN_LINK_CHANNEL_CODING 0x006
 
 #define DP_DOWN_STREAM_PORT_COUNT  0x007
 # define DP_PORT_COUNT_MASK0x0f
-# define DP_MSA_TIMING_PAR_IGNORED (1 << 6)
+# define DP_MSA_TIMING_PAR_IGNORED (1 << 6) /* eDP */
 # define DP_OUI_SUPPORT(1 << 7)
 
-#define DP_I2C_SPEED_CAP   0x00c
+#define DP_I2C_SPEED_CAP   0x00c/* DPI */
 # define DP_I2C_SPEED_1K   0x01
 # define DP_I2C_SPEED_5K   0x02
 # define DP_I2C_SPEED_10K  0x04
@@ -86,16 +98,16 @@
 # define DP_I2C_SPEED_400K 0x10
 # define DP_I2C_SPEED_1M   0x20
 
-#define DP_EDP_CONFIGURATION_CAP0x00d
-#define DP_TRAINING_AUX_RD_INTERVAL 0x00e
+#define DP_EDP_CONFIGURATION_CAP0x00d   /* XXX 1.2? */
+#define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */
 
 /* Multiple stream transport */
-#define DP_MSTM_CAP0x021
+#define DP_MSTM_CAP0x021   /* 1.2 */
 # define DP_MST_CAP(1 << 0)
 
-#define DP_PSR_SUPPORT  0x070
+#define DP_PSR_SUPPORT  0x070   /* XXX 1.2? */
 # define DP_PSR_IS_SUPPORTED1
-#define DP_PSR_CAPS 0x071
+#define DP_PSR_CAPS 0x071   /* XXX 1.2? */
 # define DP_PSR_NO_TRAIN_ON_EXIT1
 # define DP_PSR_SETUP_TIME_330  (0 << 1)
 # define DP_PSR_SETUP_TIME_275  (1 << 1)
@@ -136,7 +148,7 @@
 #defineDP_LINK_BW_SET  0x100
 # define DP_LINK_BW_1_62   0x06
 # define DP_LINK_BW_2_70x0a
-# define DP_LINK_BW_5_40x14
+# define DP_LINK_BW_5_40x14/* 1.2 */
 
 #define DP_LANE_COUNT_SET  0x101
 # define DP_LANE_COUNT_MASK0x0f
@@ -146,7 +158,7 @@
 # define DP_TRAINING_PATTERN_DISABLE   0
 # define DP_TRAINING_PATTERN_1 1
 # define DP_TRAINING_PATTERN_2 2
-# define DP_TRAINING_PATTERN_3 3
+# define DP_TRAINING_PATTERN_3 3   /* 1.2 */
 # define DP_TRAINING_PATTERN_MASK  0x3
 
 # define DP_LINK_QUAL_PATTERN_DISABLE  (0 << 2)
@@ -187,22 +199,22 @@
 
 #define DP_DOWNSPREAD_CTRL 0x107
 # define DP_SPREAD_AMP_0_5 (1 << 4)
-# define DP_MSA_TIMING_PAR_IGNORE_EN   (1 << 7)
+# define DP_MSA_TIMING_PAR_IGNORE_EN   (1 << 7) /* eDP */
 
 #define DP_MAIN_LINK_CHANNEL_CODING_SET0x108
 # define DP_SET_ANSI_8B10B (1 << 0)
 
-#define DP_I2C_SPEED_CONTROL_STATUS0x109
+#define DP_I2C_SPEED_CONTROL_STATUS0x109   /* DPI */
 /* bitmask as for DP_I2C_SPEED_CAP */
 
-#define DP_EDP_CONFIGURATION_SET0x10a
+#define DP_EDP_CONFIGURATION_SET0x10a   /* XXX 1.2? */
 
-#define DP_MSTM_CTRL   0x111
+#define DP_MSTM_CTRL   0x111   /* 1.2 */
 # define DP_MST_EN (1 << 0)
 # define DP_UP_REQ_EN  (1 << 1)
 # defin

[Intel-gfx] [PATCH 2/4] drm/dp: Update DPCD defines

[Adam Jackson]

On 9/20/12 10:10 AM, Paulo Zanoni wrote:
> 2012/9/18 Adam Jackson :
>> Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
>> 1.2 for the MST bits.
>
> All I needed to review every bit was DP spec version 1.2.

Lucky you!  I don't have a copy.

>> +#define DP_SINK_COUNT  0x200
>> +# define DP_SINK_COUNT_MASK(31 << 0)
>
> My DP spec version 1.2 says "bits 7 and 5:0", but the DP 1.1 spec says
> it's just 5:0 and "Bits 7 = RESERVED". So should we treat bit 7 as the
> most-significant-bit? Notice that this will affect patch 4 of this
> series.

Oh, wild.  I guess they did that so they could have twice as many 
downstream devices?

> Idea for a follow-up patch: maybe we should try to add some comments
> explaining which bits appeared only in some specific DPCD x.y
> revision?

That's a good idea.

I'll send follow-ups for that, and to make a DP_GET_SINK_COUNT() that 
does the right math.

- ajax

Re: [Intel-gfx] [PATCH 2/4] drm/dp: Update DPCD defines

[Adam Jackson]

On 9/20/12 10:10 AM, Paulo Zanoni wrote:

2012/9/18 Adam Jackson :

Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
1.2 for the MST bits.


All I needed to review every bit was DP spec version 1.2.


Lucky you!  I don't have a copy.


+#define DP_SINK_COUNT  0x200
+# define DP_SINK_COUNT_MASK(31 << 0)


My DP spec version 1.2 says "bits 7 and 5:0", but the DP 1.1 spec says
it's just 5:0 and "Bits 7 = RESERVED". So should we treat bit 7 as the
most-significant-bit? Notice that this will affect patch 4 of this
series.


Oh, wild.  I guess they did that so they could have twice as many 
downstream devices?



Idea for a follow-up patch: maybe we should try to add some comments
explaining which bits appeared only in some specific DPCD x.y
revision?


That's a good idea.

I'll send follow-ups for that, and to make a DP_GET_SINK_COUNT() that 
does the right math.


- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[Intel-gfx] [PATCH 4/4] drm/i915/dp: Be smarter about connection sense for branch devices

[Adam Jackson]

On Tue, 2012-09-18 at 10:58 -0400, Adam Jackson wrote:
> If there's no downstream device, DPCD success is good enough.  If
> there's a hotplug-capable downstream device, count the number of
> connected sinks in DP_SINK_STATUS and return success if it's non-zero.
> Otherwise, probe DDC and report appropriately.
> 
> v2: Check DP_SINK_STATUS instead of something unrelated to sink status.

Ugh, s/STATUS/COUNT/g in the summary, sorry.

- ajax
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: 
<http://lists.freedesktop.org/archives/dri-devel/attachments/20120918/5e75451b/attachment.pgp>

[PATCH 4/4] drm/i915/dp: Be smarter about connection sense for branch devices

[Adam Jackson]

If there's no downstream device, DPCD success is good enough.  If
there's a hotplug-capable downstream device, count the number of
connected sinks in DP_SINK_STATUS and return success if it's non-zero.
Otherwise, probe DDC and report appropriately.

v2: Check DP_SINK_STATUS instead of something unrelated to sink status.

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/i915/intel_dp.c |   35 ++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 098119e..813b771 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2088,11 +2088,44 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
}
 }

+/* XXX this is probably wrong for multiple downstream ports */
 static enum drm_connector_status
 intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 {
-   if (intel_dp_get_dpcd(intel_dp))
+   uint8_t *dpcd = intel_dp->dpcd;
+   bool hpd;
+   uint8_t type;
+
+   if (!intel_dp_get_dpcd(intel_dp))
+   return connector_status_disconnected;
+
+   /* if there's no downstream port, we're done */
+   if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+   return connector_status_connected;
+
+   /* If we're HPD-aware, SINK_COUNT changes dynamically */
+   hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD);
+   if (hpd) {
+   uint8_t sink_count;
+   if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
+   &sink_count, 1))
+   return connector_status_unknown;
+   sink_count &= DP_SINK_COUNT_MASK;
+   return sink_count ? connector_status_connected
+ : connector_status_disconnected;
+   }
+
+   /* If no HPD, poke DDC gently */
+   if (drm_probe_ddc(&intel_dp->adapter))
return connector_status_connected;
+
+   /* Well we tried, say unknown for unreliable port types */
+   type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
+   if (type == DP_DS_PORT_TYPE_VGA || type == DP_DS_PORT_TYPE_NON_EDID)
+   return connector_status_unknown;
+
+   /* Anything else is out of spec, warn and ignore */
+   DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
return connector_status_disconnected;
 }

-- 
1.7.7.6

[PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Adam Jackson]

v2: Fix parenthesis mismatch, spotted by Jani Nikula

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/i915/intel_dp.c |   25 -
 1 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ace757a..098119e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -39,6 +39,7 @@
 #include "drm_dp_helper.h"

 #define DP_RECEIVER_CAP_SIZE   0xf
+#define DP_MAX_DOWNSTREAM_PORTS 0xf
 #define DP_LINK_STATUS_SIZE6
 #define DP_LINK_CHECK_TIMEOUT  (10 * 1000)

@@ -56,6 +57,7 @@ struct intel_dp {
uint8_t link_bw;
uint8_t lane_count;
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+   uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct i2c_adapter adapter;
struct i2c_algo_dp_aux_data algo;
bool is_pch_edp;
@@ -1968,12 +1970,25 @@ static bool
 intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-  sizeof(intel_dp->dpcd)) &&
-   (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
-   return true;
-   }
+  sizeof(intel_dp->dpcd)) == 0)
+   return false; /* aux transfer failed */

-   return false;
+   if (intel_dp->dpcd[DP_DPCD_REV] == 0)
+   return false; /* DPCD not present */
+
+   if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
+ DP_DWN_STRM_PORT_PRESENT))
+   return true; /* native DP sink */
+
+   if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
+   return true; /* no per-port downstream info */
+
+   if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
+  intel_dp->downstream_ports,
+  DP_MAX_DOWNSTREAM_PORTS) == 0)
+   return false; /* downstream port status fetch failed */
+
+   return true;
 }

 static void
-- 
1.7.7.6

[PATCH 2/4] drm/dp: Update DPCD defines

[Adam Jackson]

Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
1.2 for the MST bits.

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 include/drm/drm_dp_helper.h |   60 ---
 1 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1744b18c..f9888c3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -69,16 +69,30 @@
 /* 10b = TMDS or HDMI */
 /* 11b = Other */
 # define DP_FORMAT_CONVERSION   (1 << 3)
+# define DP_DETAILED_CAP_INFO_AVAILABLE(1 << 4)

 #define DP_MAIN_LINK_CHANNEL_CODING 0x006

 #define DP_DOWN_STREAM_PORT_COUNT  0x007
-#define  DP_PORT_COUNT_MASK0x0f
-#define  DP_OUI_SUPPORT(1 << 7)
+# define DP_PORT_COUNT_MASK0x0f
+# define DP_MSA_TIMING_PAR_IGNORED (1 << 6)
+# define DP_OUI_SUPPORT(1 << 7)
+
+#define DP_I2C_SPEED_CAP   0x00c
+# define DP_I2C_SPEED_1K   0x01
+# define DP_I2C_SPEED_5K   0x02
+# define DP_I2C_SPEED_10K  0x04
+# define DP_I2C_SPEED_100K 0x08
+# define DP_I2C_SPEED_400K 0x10
+# define DP_I2C_SPEED_1M   0x20

 #define DP_EDP_CONFIGURATION_CAP0x00d
 #define DP_TRAINING_AUX_RD_INTERVAL 0x00e

+/* Multiple stream transport */
+#define DP_MSTM_CAP0x021
+# define DP_MST_CAP(1 << 0)
+
 #define DP_PSR_SUPPORT  0x070
 # define DP_PSR_IS_SUPPORTED1
 #define DP_PSR_CAPS 0x071
@@ -93,6 +107,31 @@
 # define DP_PSR_SETUP_TIME_MASK (7 << 1)
 # define DP_PSR_SETUP_TIME_SHIFT1

+/*
+ * 0x80-0x8f describe downstream port capabilities, but there are two layouts
+ * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set.  If it was not,
+ * each port's descriptor is one byte wide.  If it was set, each port's is
+ * four bytes wide, starting with the one byte from the base info.  As of
+ * DP interop v1.1a only VGA defines additional detail.
+ */
+
+/* offset 0 */
+#define DP_DOWNSTREAM_PORT_0   0x80
+# define DP_DS_PORT_TYPE_MASK  (7 << 0)
+# define DP_DS_PORT_TYPE_DP0
+# define DP_DS_PORT_TYPE_VGA   1
+# define DP_DS_PORT_TYPE_DVI   2
+# define DP_DS_PORT_TYPE_HDMI  3
+# define DP_DS_PORT_TYPE_NON_EDID  4
+# define DP_DS_PORT_HPD(1 << 3)
+/* offset 1 for VGA is maximum megapixels per second / 8 */
+/* offset 2 */
+# define DP_DS_VGA_MAX_BPC_MASK(3 << 0)
+# define DP_DS_VGA_8BPC0
+# define DP_DS_VGA_10BPC   1
+# define DP_DS_VGA_12BPC   2
+# define DP_DS_VGA_16BPC   3
+
 /* link configuration */
 #defineDP_LINK_BW_SET  0x100
 # define DP_LINK_BW_1_62   0x06
@@ -148,24 +187,37 @@

 #define DP_DOWNSPREAD_CTRL 0x107
 # define DP_SPREAD_AMP_0_5 (1 << 4)
+# define DP_MSA_TIMING_PAR_IGNORE_EN   (1 << 7)

 #define DP_MAIN_LINK_CHANNEL_CODING_SET0x108
 # define DP_SET_ANSI_8B10B (1 << 0)

+#define DP_I2C_SPEED_CONTROL_STATUS0x109
+/* bitmask as for DP_I2C_SPEED_CAP */
+
+#define DP_EDP_CONFIGURATION_SET0x10a
+
+#define DP_MSTM_CTRL   0x111
+# define DP_MST_EN (1 << 0)
+# define DP_UP_REQ_EN  (1 << 1)
+# define DP_UPSTREAM_IS_SRC(1 << 2)
+
 #define DP_PSR_EN_CFG  0x170
 # define DP_PSR_ENABLE (1 << 0)
 # define DP_PSR_MAIN_LINK_ACTIVE   (1 << 1)
 # define DP_PSR_CRC_VERIFICATION   (1 << 2)
 # define DP_PSR_FRAME_CAPTURE  (1 << 3)

+#define DP_SINK_COUNT  0x200
+# define DP_SINK_COUNT_MASK(31 << 0)
+# define DP_SINK_CP_READY  (1 << 6)
+
 #define DP_DEVICE_SERVICE_IRQ_VECTOR   0x201
 # define DP_REMOTE_CONTROL_COMMAND_PENDING  (1 << 0)
 # define DP_AUTOMATED_TEST_REQUEST (1 << 1)
 # define DP_CP_IRQ (1 << 2)
 # define DP_SINK_SPECIFIC_IRQ  (1 << 6)

-#define DP_EDP_CONFIGURATION_SET0x10a
-
 #define DP_LANE0_1_STATUS  0x202
 #define DP_LANE2_3_STATUS  0x203
 # define DP_LANE_CR_DONE   (1 << 0)
-- 
1.7.7.6

[PATCH 1/4] drm: Export drm_probe_ddc()

[Adam Jackson]

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/drm_edid.c |3 ++-
 include/drm/drm_crtc.h |1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b7ee230..a1895ba 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -376,13 +376,14 @@ out:
  * \param adapter : i2c device adaptor
  * \return 1 on success
  */
-static bool
+bool
 drm_probe_ddc(struct i2c_adapter *adapter)
 {
unsigned char out;

return (drm_do_probe_ddc_edid(adapter, &out, 0, 1) == 0);
 }
+EXPORT_SYMBOL(drm_probe_ddc);

 /**
  * drm_get_edid - get EDID data, if available
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bfacf0d..f5d434b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -862,6 +862,7 @@ extern char *drm_get_tv_subconnector_name(int val);
 extern char *drm_get_tv_select_name(int val);
 extern void drm_fb_release(struct drm_file *file_priv);
 extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct 
drm_mode_group *group);
+extern bool drm_probe_ddc(struct i2c_adapter *adapter);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 struct i2c_adapter *adapter);
 extern int drm_add_edid_modes(struct drm_connector *connector, struct edid 
*edid);
-- 
1.7.7.6

Re: [Intel-gfx] [PATCH 4/4] drm/i915/dp: Be smarter about connection sense for branch devices

[Adam Jackson]

On Tue, 2012-09-18 at 10:58 -0400, Adam Jackson wrote:
> If there's no downstream device, DPCD success is good enough.  If
> there's a hotplug-capable downstream device, count the number of
> connected sinks in DP_SINK_STATUS and return success if it's non-zero.
> Otherwise, probe DDC and report appropriately.
> 
> v2: Check DP_SINK_STATUS instead of something unrelated to sink status.

Ugh, s/STATUS/COUNT/g in the summary, sorry.

- ajax


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/i915/dp: Be smarter about connection sense for branch devices

[Adam Jackson]

If there's no downstream device, DPCD success is good enough.  If
there's a hotplug-capable downstream device, count the number of
connected sinks in DP_SINK_STATUS and return success if it's non-zero.
Otherwise, probe DDC and report appropriately.

v2: Check DP_SINK_STATUS instead of something unrelated to sink status.

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/i915/intel_dp.c |   35 ++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 098119e..813b771 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2088,11 +2088,44 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
}
 }
 
+/* XXX this is probably wrong for multiple downstream ports */
 static enum drm_connector_status
 intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 {
-   if (intel_dp_get_dpcd(intel_dp))
+   uint8_t *dpcd = intel_dp->dpcd;
+   bool hpd;
+   uint8_t type;
+
+   if (!intel_dp_get_dpcd(intel_dp))
+   return connector_status_disconnected;
+
+   /* if there's no downstream port, we're done */
+   if (!(dpcd[DP_DOWNSTREAMPORT_PRESENT] & DP_DWN_STRM_PORT_PRESENT))
+   return connector_status_connected;
+
+   /* If we're HPD-aware, SINK_COUNT changes dynamically */
+   hpd = !!(intel_dp->downstream_ports[0] & DP_DS_PORT_HPD);
+   if (hpd) {
+   uint8_t sink_count;
+   if (!intel_dp_aux_native_read_retry(intel_dp, DP_SINK_COUNT,
+   &sink_count, 1))
+   return connector_status_unknown;
+   sink_count &= DP_SINK_COUNT_MASK;
+   return sink_count ? connector_status_connected
+ : connector_status_disconnected;
+   }
+
+   /* If no HPD, poke DDC gently */
+   if (drm_probe_ddc(&intel_dp->adapter))
return connector_status_connected;
+
+   /* Well we tried, say unknown for unreliable port types */
+   type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
+   if (type == DP_DS_PORT_TYPE_VGA || type == DP_DS_PORT_TYPE_NON_EDID)
+   return connector_status_unknown;
+
+   /* Anything else is out of spec, warn and ignore */
+   DRM_DEBUG_KMS("Broken DP branch device, ignoring\n");
return connector_status_disconnected;
 }
 
-- 
1.7.7.6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/4] drm/i915/dp: Fetch downstream port info if needed during DPCD fetch

[Adam Jackson]

v2: Fix parenthesis mismatch, spotted by Jani Nikula

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/i915/intel_dp.c |   25 -
 1 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index ace757a..098119e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -39,6 +39,7 @@
 #include "drm_dp_helper.h"
 
 #define DP_RECEIVER_CAP_SIZE   0xf
+#define DP_MAX_DOWNSTREAM_PORTS 0xf
 #define DP_LINK_STATUS_SIZE6
 #define DP_LINK_CHECK_TIMEOUT  (10 * 1000)
 
@@ -56,6 +57,7 @@ struct intel_dp {
uint8_t link_bw;
uint8_t lane_count;
uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
+   uint8_t downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
struct i2c_adapter adapter;
struct i2c_algo_dp_aux_data algo;
bool is_pch_edp;
@@ -1968,12 +1970,25 @@ static bool
 intel_dp_get_dpcd(struct intel_dp *intel_dp)
 {
if (intel_dp_aux_native_read_retry(intel_dp, 0x000, intel_dp->dpcd,
-  sizeof(intel_dp->dpcd)) &&
-   (intel_dp->dpcd[DP_DPCD_REV] != 0)) {
-   return true;
-   }
+  sizeof(intel_dp->dpcd)) == 0)
+   return false; /* aux transfer failed */
 
-   return false;
+   if (intel_dp->dpcd[DP_DPCD_REV] == 0)
+   return false; /* DPCD not present */
+
+   if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
+ DP_DWN_STRM_PORT_PRESENT))
+   return true; /* native DP sink */
+
+   if (intel_dp->dpcd[DP_DPCD_REV] == 0x10)
+   return true; /* no per-port downstream info */
+
+   if (intel_dp_aux_native_read_retry(intel_dp, DP_DOWNSTREAM_PORT_0,
+  intel_dp->downstream_ports,
+  DP_MAX_DOWNSTREAM_PORTS) == 0)
+   return false; /* downstream port status fetch failed */
+
+   return true;
 }
 
 static void
-- 
1.7.7.6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/dp: Update DPCD defines

[Adam Jackson]

Sources: DP, eDP, and DP interop specs, and a VESA slideshow about DP
1.2 for the MST bits.

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 include/drm/drm_dp_helper.h |   60 ---
 1 files changed, 56 insertions(+), 4 deletions(-)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 1744b18c..f9888c3 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -69,16 +69,30 @@
 /* 10b = TMDS or HDMI */
 /* 11b = Other */
 # define DP_FORMAT_CONVERSION   (1 << 3)
+# define DP_DETAILED_CAP_INFO_AVAILABLE(1 << 4)
 
 #define DP_MAIN_LINK_CHANNEL_CODING 0x006
 
 #define DP_DOWN_STREAM_PORT_COUNT  0x007
-#define  DP_PORT_COUNT_MASK0x0f
-#define  DP_OUI_SUPPORT(1 << 7)
+# define DP_PORT_COUNT_MASK0x0f
+# define DP_MSA_TIMING_PAR_IGNORED (1 << 6)
+# define DP_OUI_SUPPORT(1 << 7)
+
+#define DP_I2C_SPEED_CAP   0x00c
+# define DP_I2C_SPEED_1K   0x01
+# define DP_I2C_SPEED_5K   0x02
+# define DP_I2C_SPEED_10K  0x04
+# define DP_I2C_SPEED_100K 0x08
+# define DP_I2C_SPEED_400K 0x10
+# define DP_I2C_SPEED_1M   0x20
 
 #define DP_EDP_CONFIGURATION_CAP0x00d
 #define DP_TRAINING_AUX_RD_INTERVAL 0x00e
 
+/* Multiple stream transport */
+#define DP_MSTM_CAP0x021
+# define DP_MST_CAP(1 << 0)
+
 #define DP_PSR_SUPPORT  0x070
 # define DP_PSR_IS_SUPPORTED1
 #define DP_PSR_CAPS 0x071
@@ -93,6 +107,31 @@
 # define DP_PSR_SETUP_TIME_MASK (7 << 1)
 # define DP_PSR_SETUP_TIME_SHIFT1
 
+/*
+ * 0x80-0x8f describe downstream port capabilities, but there are two layouts
+ * based on whether DP_DETAILED_CAP_INFO_AVAILABLE was set.  If it was not,
+ * each port's descriptor is one byte wide.  If it was set, each port's is
+ * four bytes wide, starting with the one byte from the base info.  As of
+ * DP interop v1.1a only VGA defines additional detail.
+ */
+
+/* offset 0 */
+#define DP_DOWNSTREAM_PORT_0   0x80
+# define DP_DS_PORT_TYPE_MASK  (7 << 0)
+# define DP_DS_PORT_TYPE_DP0
+# define DP_DS_PORT_TYPE_VGA   1
+# define DP_DS_PORT_TYPE_DVI   2
+# define DP_DS_PORT_TYPE_HDMI  3
+# define DP_DS_PORT_TYPE_NON_EDID  4
+# define DP_DS_PORT_HPD(1 << 3)
+/* offset 1 for VGA is maximum megapixels per second / 8 */
+/* offset 2 */
+# define DP_DS_VGA_MAX_BPC_MASK(3 << 0)
+# define DP_DS_VGA_8BPC0
+# define DP_DS_VGA_10BPC   1
+# define DP_DS_VGA_12BPC   2
+# define DP_DS_VGA_16BPC   3
+
 /* link configuration */
 #defineDP_LINK_BW_SET  0x100
 # define DP_LINK_BW_1_62   0x06
@@ -148,24 +187,37 @@
 
 #define DP_DOWNSPREAD_CTRL 0x107
 # define DP_SPREAD_AMP_0_5 (1 << 4)
+# define DP_MSA_TIMING_PAR_IGNORE_EN   (1 << 7)
 
 #define DP_MAIN_LINK_CHANNEL_CODING_SET0x108
 # define DP_SET_ANSI_8B10B (1 << 0)
 
+#define DP_I2C_SPEED_CONTROL_STATUS0x109
+/* bitmask as for DP_I2C_SPEED_CAP */
+
+#define DP_EDP_CONFIGURATION_SET0x10a
+
+#define DP_MSTM_CTRL   0x111
+# define DP_MST_EN (1 << 0)
+# define DP_UP_REQ_EN  (1 << 1)
+# define DP_UPSTREAM_IS_SRC(1 << 2)
+
 #define DP_PSR_EN_CFG  0x170
 # define DP_PSR_ENABLE (1 << 0)
 # define DP_PSR_MAIN_LINK_ACTIVE   (1 << 1)
 # define DP_PSR_CRC_VERIFICATION   (1 << 2)
 # define DP_PSR_FRAME_CAPTURE  (1 << 3)
 
+#define DP_SINK_COUNT  0x200
+# define DP_SINK_COUNT_MASK(31 << 0)
+# define DP_SINK_CP_READY  (1 << 6)
+
 #define DP_DEVICE_SERVICE_IRQ_VECTOR   0x201
 # define DP_REMOTE_CONTROL_COMMAND_PENDING  (1 << 0)
 # define DP_AUTOMATED_TEST_REQUEST (1 << 1)
 # define DP_CP_IRQ (1 << 2)
 # define DP_SINK_SPECIFIC_IRQ  (1 << 6)
 
-#define DP_EDP_CONFIGURATION_SET0x10a
-
 #define DP_LANE0_1_STATUS  0x202
 #define DP_LANE2_3_STATUS  0x203
 # define DP_LANE_CR_DONE   (1 << 0)
-- 
1.7.7.6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm: Export drm_probe_ddc()

[Adam Jackson]

Tested-by: Takashi Iwai 
Signed-off-by: Adam Jackson 
---
 drivers/gpu/drm/drm_edid.c |3 ++-
 include/drm/drm_crtc.h |1 +
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b7ee230..a1895ba 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -376,13 +376,14 @@ out:
  * \param adapter : i2c device adaptor
  * \return 1 on success
  */
-static bool
+bool
 drm_probe_ddc(struct i2c_adapter *adapter)
 {
unsigned char out;
 
return (drm_do_probe_ddc_edid(adapter, &out, 0, 1) == 0);
 }
+EXPORT_SYMBOL(drm_probe_ddc);
 
 /**
  * drm_get_edid - get EDID data, if available
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bfacf0d..f5d434b 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -862,6 +862,7 @@ extern char *drm_get_tv_subconnector_name(int val);
 extern char *drm_get_tv_select_name(int val);
 extern void drm_fb_release(struct drm_file *file_priv);
 extern int drm_mode_group_init_legacy_group(struct drm_device *dev, struct 
drm_mode_group *group);
+extern bool drm_probe_ddc(struct i2c_adapter *adapter);
 extern struct edid *drm_get_edid(struct drm_connector *connector,
 struct i2c_adapter *adapter);
 extern int drm_add_edid_modes(struct drm_connector *connector, struct edid 
*edid);
-- 
1.7.7.6

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V6] drm: edid: add support for E-DDC

[Adam Jackson]

On 9/8/12 6:35 AM, Dave Airlie wrote:
> On Sat, Sep 8, 2012 at 3:30 AM, Shirish S  wrote:
>> Hi Dave,
>> Gentle Reminder!
>> This patch is required for passing the very first test case of HDMI
>> Compliance test suite.
>> Regards,
>> Shirish S
>
> Can you provide ajax with the sample he asked for previously?

He did, it even looks like I expected it to look: a (correct) block map 
and two CEA extension blocks.  Which I think means there are now ways 
two successive CEA blocks could disagree with each other, so that's 
probably something to audit, but I don't expect that to be common.

- ajax

Re: [PATCH V6] drm: edid: add support for E-DDC

[Adam Jackson]

On 9/8/12 6:35 AM, Dave Airlie wrote:

On Sat, Sep 8, 2012 at 3:30 AM, Shirish S  wrote:

Hi Dave,
Gentle Reminder!
This patch is required for passing the very first test case of HDMI
Compliance test suite.
Regards,
Shirish S


Can you provide ajax with the sample he asked for previously?


He did, it even looks like I expected it to look: a (correct) block map 
and two CEA extension blocks.  Which I think means there are now ways 
two successive CEA blocks could disagree with each other, so that's 
probably something to audit, but I don't expect that to be common.


- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

PCI Radeon RV100 detection hang on sparc64

[Adam Jackson]

On Thu, 2012-09-06 at 17:41 +0300, Meelis Roos wrote:

> [drm] radeon defaulting to kernel modesetting.
> [drm] radeon kernel modesetting enabled.
> PCI: Enabling device: (:02:02.0), cmd 82
> [drm] initializing kernel modesetting (RV100 0x1002:0x5159 0x1002:0x0908).
> [drm] register mmio base: 0x1000
> [drm] register mmio size: 32768
> [drm:radeon_device_init] *ERROR* Unable to find PCI I/O BAR

This particular message looks like it might actually be harmless, all
the code paths that use the I/O BAR can use the MMIO mirror of it
instead.  No idea how well tested that is, but at any rate it's not
what's breaking your setup.

> And here the machine hangs. Debugging printk-s reveal that it does not 
> find any active I/O port resources and then continues into initializing 
> the card. Down in igp_read_bios_from_vram() it successfully ioremaps 
> memory region 0 (vram_base=1ff0800 and size=4) and tries to read 
> 2 bytes from there and hangs on reading bios[0].

This is probably because...

> 02:02.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI RV100 
> QY [Radeon 7000/VE] (prog-if 00 [VGA controller])
> Subsystem: Advanced Micro Devices [AMD] nee ATI XVR-100 (supplied by 
> Sun)
> Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping+ SERR- FastB2B- DisINTx-
> Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
> SERR-  Latency: 0 (2000ns min)
> Interrupt: pin A routed to IRQ 16
> Region 0: Memory at 0800 (32-bit, prefetchable) [size=64M]
> Region 1: I/O ports at 0400 [disabled] [size=256]
> Region 2: Memory at 1000 (32-bit, non-prefetchable) [size=32K]
> Region 3: [virtual] Memory at fe01 (32-bit, 
> non-prefetchable) [size=1]
> Region 4: [virtual] Memory at fe01 (32-bit, 
> non-prefetchable) [size=1]
> Region 5: [virtual] Memory at fe01 (32-bit, 
> non-prefetchable) [size=1]
> Expansion ROM at 1002 [disabled] [size=128K]

... the ROM BAR looks like it's not routed.

On x86 you could fix this by booting with 'pci=rom' to force the ROM BAR
to be routed regardless of firmware setup, no idea how that's meant to
work on sparc though.

- ajax
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: 

Re: PCI Radeon RV100 detection hang on sparc64

[Adam Jackson]

On Thu, 2012-09-06 at 17:41 +0300, Meelis Roos wrote:

> [drm] radeon defaulting to kernel modesetting.
> [drm] radeon kernel modesetting enabled.
> PCI: Enabling device: (:02:02.0), cmd 82
> [drm] initializing kernel modesetting (RV100 0x1002:0x5159 0x1002:0x0908).
> [drm] register mmio base: 0x1000
> [drm] register mmio size: 32768
> [drm:radeon_device_init] *ERROR* Unable to find PCI I/O BAR

This particular message looks like it might actually be harmless, all
the code paths that use the I/O BAR can use the MMIO mirror of it
instead.  No idea how well tested that is, but at any rate it's not
what's breaking your setup.

> And here the machine hangs. Debugging printk-s reveal that it does not 
> find any active I/O port resources and then continues into initializing 
> the card. Down in igp_read_bios_from_vram() it successfully ioremaps 
> memory region 0 (vram_base=1ff0800 and size=4) and tries to read 
> 2 bytes from there and hangs on reading bios[0].

This is probably because...

> 02:02.0 VGA compatible controller: Advanced Micro Devices [AMD] nee ATI RV100 
> QY [Radeon 7000/VE] (prog-if 00 [VGA controller])
> Subsystem: Advanced Micro Devices [AMD] nee ATI XVR-100 (supplied by 
> Sun)
> Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
> Stepping+ SERR- FastB2B- DisINTx-
> Status: Cap+ 66MHz- UDF- FastB2B+ ParErr- DEVSEL=medium >TAbort- 
> SERR-  Latency: 0 (2000ns min)
> Interrupt: pin A routed to IRQ 16
> Region 0: Memory at 0800 (32-bit, prefetchable) [size=64M]
> Region 1: I/O ports at 0400 [disabled] [size=256]
> Region 2: Memory at 1000 (32-bit, non-prefetchable) [size=32K]
> Region 3: [virtual] Memory at fe01 (32-bit, 
> non-prefetchable) [size=1]
> Region 4: [virtual] Memory at fe01 (32-bit, 
> non-prefetchable) [size=1]
> Region 5: [virtual] Memory at fe01 (32-bit, 
> non-prefetchable) [size=1]
> Expansion ROM at 1002 [disabled] [size=128K]

... the ROM BAR looks like it's not routed.

On x86 you could fix this by booting with 'pci=rom' to force the ROM BAR
to be routed regardless of firmware setup, no idea how that's meant to
work on sparc though.

- ajax


signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH V6] drm: edid: add support for E-DDC

[Adam Jackson]

On 9/3/12 11:54 AM, Shirish S wrote:
> Hello Dave,
>
> My patch-set for adding support for 4 block EDID is now reviewed and ready.
> Please let me know if you want any further clarification

I assume you have actual displays with that many EDID extensions.  Can 
you send a sample of the complete EDID block from one such display?  I'd 
be interested to know what extensions are present.

- ajax

Re: [PATCH V6] drm: edid: add support for E-DDC

[Adam Jackson]

On 9/3/12 11:54 AM, Shirish S wrote:

Hello Dave,

My patch-set for adding support for 4 block EDID is now reviewed and ready.
Please let me know if you want any further clarification


I assume you have actual displays with that many EDID extensions.  Can 
you send a sample of the complete EDID block from one such display?  I'd 
be interested to know what extensions are present.


- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/3] drm/edid: Be stricter about stereo mode rejection

[Adam Jackson]

On 6/6/12 3:07 PM, Adam Jackson wrote:
> Either bit 5 or 6 of that byte may be set in a stereo mode.
>
> E-EDID v1.4, Table 3.22
>
> Signed-off-by: Adam Jackson 

Anyone want to give this series some love?

- ajax

Re: [PATCH 1/3] drm/edid: Be stricter about stereo mode rejection

[Adam Jackson]

On 6/6/12 3:07 PM, Adam Jackson wrote:

Either bit 5 or 6 of that byte may be set in a stereo mode.

E-EDID v1.4, Table 3.22

Signed-off-by: Adam Jackson 


Anyone want to give this series some love?

- ajax

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/i915/hdmi: only enable audio if there's a hdmi sink

[Adam Jackson]

On 8/30/12 3:44 AM, Daniel Vetter wrote:
> Some monitors totally don't like to receive infoframes, and naturally
> don't claim to support hdmi.
>
> But for some odd reason they've added a CEA block to their edid,
> which automatically gives you basic audio. Still, we may not send
> out hdmi infoframes to them, hence check whether the sink is indeed
> hdmi capable.
>
> Also kill a stale comment while at it.

Does an incomplete version of what it says on the box, you need to hit 
the HDMI code in intel_sdvo.c too.

At which point, should do the obvious bool drm_monitor_has_hdmi_audio() 
in drm_edid.c.

- ajax

Re: [PATCH] drm/i915/hdmi: only enable audio if there's a hdmi sink

[Adam Jackson]

On 8/30/12 3:44 AM, Daniel Vetter wrote:

Some monitors totally don't like to receive infoframes, and naturally
don't claim to support hdmi.

But for some odd reason they've added a CEA block to their edid,
which automatically gives you basic audio. Still, we may not send
out hdmi infoframes to them, hence check whether the sink is indeed
hdmi capable.

Also kill a stale comment while at it.


Does an incomplete version of what it says on the box, you need to hit 
the HDMI code in intel_sdvo.c too.


At which point, should do the obvious bool drm_monitor_has_hdmi_audio() 
in drm_edid.c.


- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

-next queue and EDID stuff

[Adam Jackson]

On 8/29/12 3:53 PM, Ian Pilcher wrote:

> Thinking a bit more about this, I'm starting to rethink my assertion
> that the Intel driver is at fault here.  On one hand, it doesn't make
> any sense to send audio InfoFrames to a non-HDMI display.  (BTW, I'm
> assuming that a display with a DisplayPort port will show up as HDMI.)

Nope.

If you connect to the DP port on a monitor, it will claim to be DP: the 
EDID block version will be 1.4, and the digital display feature byte 
will indicate that it's DP:

http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n1145

If you connect to the HDMI port on a monitor, it will claim to be HDMI: 
the EDID block version will be 1.3 or higher, there will be a CEA 
extension block, and there will be a vendor-specific data block within 
that extension with a vendor OUI matching that of the HDMI consortium:

http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n640

> On the other hand, it can be argued that the DRM layer is giving
> conflicting information to the driver -- drm_detect_hdmi_monitor is
> returning FALSE, but drm_detect_monitor_audio is returning TRUE.  AFAIK,
> this doesn't make any sense either.

This might be the actual issue.  drm_detect_monitor_audio() can return 
true for several reasons, but it does _not_ make any claim that the 
monitor is HDMI, only that it has a CEA extension block.  Which is 
certainly ugly, but that's how CEA defined their botch of an extension 
block.

So to me, that says that drivers need to ask _both_ whether the monitor 
supports audio and whether it's HDMI before sending HDMI-specific audio 
signalling.

- ajax

Re: -next queue and EDID stuff

[Adam Jackson]

On 8/29/12 3:53 PM, Ian Pilcher wrote:


Thinking a bit more about this, I'm starting to rethink my assertion
that the Intel driver is at fault here.  On one hand, it doesn't make
any sense to send audio InfoFrames to a non-HDMI display.  (BTW, I'm
assuming that a display with a DisplayPort port will show up as HDMI.)


Nope.

If you connect to the DP port on a monitor, it will claim to be DP: the 
EDID block version will be 1.4, and the digital display feature byte 
will indicate that it's DP:


http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n1145

If you connect to the HDMI port on a monitor, it will claim to be HDMI: 
the EDID block version will be 1.3 or higher, there will be a CEA 
extension block, and there will be a vendor-specific data block within 
that extension with a vendor OUI matching that of the HDMI consortium:


http://cgit.freedesktop.org/xorg/app/edid-decode/tree/edid-decode.c#n640


On the other hand, it can be argued that the DRM layer is giving
conflicting information to the driver -- drm_detect_hdmi_monitor is
returning FALSE, but drm_detect_monitor_audio is returning TRUE.  AFAIK,
this doesn't make any sense either.


This might be the actual issue.  drm_detect_monitor_audio() can return 
true for several reasons, but it does _not_ make any claim that the 
monitor is HDMI, only that it has a CEA extension block.  Which is 
certainly ugly, but that's how CEA defined their botch of an extension 
block.


So to me, that says that drivers need to ask _both_ whether the monitor 
supports audio and whether it's HDMI before sending HDMI-specific audio 
signalling.


- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

-next queue and EDID stuff

[Adam Jackson]

On 8/28/12 7:33 PM, Ian Pilcher wrote:
> On 08/27/2012 03:24 PM, Adam Jackson wrote:
>> https://patchwork.kernel.org/patch/1310091/ - nak, the commit message
>> itself gives away that we're doing else something wrong here.  If
>> DISABLE_AUDIO is sufficient for one driver it should be sufficient for all.
>
> Actually, I believe that the error is probably in the Intel driver.  As
> I understand it, it shouldn't be sending audio InfoFrames to a non-HDMI
> display.

If that's the case then I'd still say "we're doing something else wrong 
here".  Quirks - at least at the core drm level - are not for working 
around broken drivers, they're for working around broken displays.

- ajax

Re: -next queue and EDID stuff

[Adam Jackson]

On 8/28/12 7:33 PM, Ian Pilcher wrote:

On 08/27/2012 03:24 PM, Adam Jackson wrote:

https://patchwork.kernel.org/patch/1310091/ - nak, the commit message
itself gives away that we're doing else something wrong here.  If
DISABLE_AUDIO is sufficient for one driver it should be sufficient for all.


Actually, I believe that the error is probably in the Intel driver.  As
I understand it, it shouldn't be sending audio InfoFrames to a non-HDMI
display.


If that's the case then I'd still say "we're doing something else wrong 
here".  Quirks - at least at the core drm level - are not for working 
around broken drivers, they're for working around broken displays.


- ajax
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel