[Intel-gfx] [PATCH] drm/i915/psr: disable psr2 for resolution greater than 32X20

[Rodrigo Vivi]

patch merged to dinq. thanks for the patch

On Tue, Jan 10, 2017 at 12:48 PM Vivi, Rodrigo 
wrote:

> Reviewed-by: Rodrigo Vivi 
>
> On Tue, 2017-01-10 at 12:32 +0530, vathsala nagaraju wrote:
> > PSR2 is restricted to work with panel resolutions upto 3200x2000,
> > move the check to intel_psr_match_conditions and fully block psr.
> >
> > Cc: Rodrigo Vivi 
> > Cc: Jim Bride 
> > Suggested-by: Rodrigo Vivi 
> > Signed-off-by: Vathsala Nagaraju 
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 15 ---
> >  1 file changed, 8 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> > index 6aca8ff..f2ca2a9 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -387,6 +387,13 @@ static bool intel_psr_match_conditions(struct
> intel_dp *intel_dp)
> >   return false;
> >   }
> >
> > + /* PSR2 is restricted to work with panel resolutions upto
> 3200x2000 */
> > + if (intel_crtc->config->pipe_src_w > 3200 ||
> > + intel_crtc->config->pipe_src_h > 2000) {
> > + dev_priv->psr.psr2_support = false;
> > + return false;
> > + }
> > +
> >   dev_priv->psr.source_ok = true;
> >   return true;
> >  }
> > @@ -425,7 +432,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
> >   struct intel_digital_port *intel_dig_port =
> dp_to_dig_port(intel_dp);
> >   struct drm_device *dev = intel_dig_port->base.base.dev;
> >   struct drm_i915_private *dev_priv = to_i915(dev);
> > - struct intel_crtc *crtc =
> to_intel_crtc(intel_dig_port->base.base.crtc);
> >
> >   if (!HAS_PSR(dev_priv)) {
> >   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> > @@ -452,12 +458,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
> >   hsw_psr_setup_vsc(intel_dp);
> >
> >   if (dev_priv->psr.psr2_support) {
> > - /* PSR2 is restricted to work with panel
> resolutions upto 3200x2000 */
> > - if (crtc->config->pipe_src_w > 3200 ||
> > - crtc->config->pipe_src_h > 2000)
> > - dev_priv->psr.psr2_support = false;
> > - else
> > - skl_psr_setup_su_vsc(intel_dp);
> > + skl_psr_setup_su_vsc(intel_dp);
> >   }
> >
> >   /*
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/b1bb6593/attachment-0001.html>

[Intel-gfx] [PATCH 02/10] drm/i915/psr: program vsc header for psr2

[Rodrigo Vivi]

eader.HB2 = 0x3;
> > - psr_vsc.sdp_header.HB3 = 0xb;
> > + if (dev_priv->psr.colorimetry_support &&
> > + dev_priv->psr.y_cord_support) {
> > + psr_vsc.sdp_header.HB2 = 0x5;
> > + psr_vsc.sdp_header.HB3 = 0x13;
> > + } else if (dev_priv->psr.y_cord_support) {
> > + psr_vsc.sdp_header.HB2 = 0x4;
> > + psr_vsc.sdp_header.HB3 = 0xe;
> > + } else {
> > + psr_vsc.sdp_header.HB2 = 0x3;
> > + psr_vsc.sdp_header.HB3 = 0xc;
> > + }
> > +
> >   intel_psr_write_vsc(intel_dp, _vsc);
> >  }
> >
> > --
> > 1.9.1
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/9261be42/attachment.html>

[Intel-gfx] [PATCH 01/10] drm : adds Y-coordinate and Colorimetry Format

[Rodrigo Vivi]

patch merged to dinq. thanks for patch and review.

On Mon, Jan 2, 2017 at 3:34 AM vathsala nagaraju <
vathsala.nagaraju at intel.com> wrote:

> PSR2 vsc revision number hb2( as per table 6-11)is updated to
> 4 or 5 based on Y cordinate and Colorimetry Format as below
> 04h = 3D stereo + PSR/PSR2 + Y-coordinate.
> 05h = -3D stereo- + PSR/PSR2 + Y-coordinate + Pixel Encoding/Colorimetry
> Format indication. A DP Source device is allowed to indicate the pixel
> encoding/colorimetry format to the DP Sink device with VSC SDP only when
> the DP Sink device supports it (
> i.e.,VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED bit in the
> DPRX_FEATURE_ENUMERATION_LIST register (DPCD Address 02210h, bit 3;
> is set to 1).
>
> v2: (Jani)
> - Change DP_PSR_Y_COORDINATE to DP_PSR2_SU_Y_COORDINATE_REQUIRED.
> - Add DP_PSR2_SU_GRANULARITY_REQUIRED.
> - Change DPRX_FEATURE_ENUMERATION_LIST to DP_DPRX.
> - Add GTC_CAP and AV_SYNC_CAP, other bits in DPRX_FEATURE_ENUMERATION_LIST.
>
> v3: (Jani)
> - Add support for bits 7:4 and 1 as per DP v1.4 for
>   DPRX_FEATURE_ENUMERATION_LIST.
>
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Patil Deepti 
> Reviewed-by: Jani Nikula 
> ---
>  include/drm/drm_dp_helper.h | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 55bbeb0..0468135 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -194,7 +194,8 @@
>  # define DP_PSR_SETUP_TIME_0(6 << 1)
>  # define DP_PSR_SETUP_TIME_MASK (7 << 1)
>  # define DP_PSR_SETUP_TIME_SHIFT1
> -
> +# define DP_PSR2_SU_Y_COORDINATE_REQUIRED   (1 << 4)  /* eDP 1.4a */
> +# define DP_PSR2_SU_GRANULARITY_REQUIRED(1 << 5)  /* eDP 1.4b */
>  /*
>   * 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,
> @@ -568,6 +569,16 @@
>  #define DP_RECEIVER_ALPM_STATUS0x200b  /* eDP 1.4 */
>  # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0)
>
> +#define DP_DPRX_FEATURE_ENUMERATION_LIST0x2210  /* DP 1.3 */
> +# define DP_GTC_CAP(1 << 0)  /* DP
> 1.3 */
> +# define DP_SST_SPLIT_SDP_CAP  (1 << 1)  /* DP
> 1.4 */
> +# define DP_AV_SYNC_CAP(1 << 2)
> /* DP 1.3 */
> +# define DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED  (1 << 3)  /* DP
> 1.3 */
> +# define DP_VSC_EXT_VESA_SDP_SUPPORTED (1 << 4)  /* DP
> 1.4 */
> +# define DP_VSC_EXT_VESA_SDP_CHAINING_SUPPORTED(1 << 5)
> /* DP 1.4 */
> +# define DP_VSC_EXT_CEA_SDP_SUPPORTED  (1 << 6)  /* DP
> 1.4 */
> +# define DP_VSC_EXT_CEA_SDP_CHAINING_SUPPORTED (1 << 7)  /* DP
> 1.4 */
> +
>  /* DP 1.2 Sideband message defines */
>  /* peer device type - DP 1.2a Table 2-92 */
>  #define DP_PEER_DEVICE_NONE0x0
> --
> 1.9.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/0a420103/attachment-0001.html>

[PATCH V7 1/4] Documentation/devicetree/bindings: b850v3_lvds_dp

[Laurent Pinchart]

Hi Peter,

Thank you for the patch.

On Sunday 01 Jan 2017 21:24:29 Peter Senna Tschudin wrote:
> Devicetree bindings documentation for the GE B850v3 LVDS/DP++
> display bridge.
> 
> Cc: Martyn Welch 
> Cc: Martin Donnelly 
> Cc: Javier Martinez Canillas 
> Cc: Enric Balletbo i Serra 
> Cc: Philipp Zabel 
> Cc: Rob Herring 
> Cc: Fabio Estevam 
> Signed-off-by: Peter Senna Tschudin 
> ---
> There was an Acked-by from Rob Herring  for V6, but I
> changed the bindings to use i2c_new_secondary_device() so I removed it from
> the commit message.
> 
>  .../devicetree/bindings/ge/b850v3-lvds-dp.txt  | 39 +++
>  1 file changed, 39 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
> 
> diff --git a/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
> b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt new file mode
> 100644
> index 000..1bc6ebf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
> @@ -0,0 +1,39 @@
> +Driver for GE B850v3 LVDS/DP++ display bridge
> +
> +Required properties:
> +  - compatible : should be "ge,b850v3-lvds-dp".
> +  - reg : should contain the main address which is used to ack the
> +interrupts and address for edid.
> +  - reg-names : comma separeted list of register names. Valid values
> +are "main", and "edid".
> +  - interrupt-parent : phandle of the interrupt controller that services
> +interrupts to the device
> +  - interrupts : one interrupt should be described here, as in
> +<0 IRQ_TYPE_LEVEL_HIGH>.
> +  - port : should describe the video signal connection between the host
> +and the bridge.

A bridge has, by definition, (at least) one input and (at least) one output. 
You should thus have (at least) two ports.

> +Example:
> +
> +_i2c2 {
> + status = "okay";
> + clock-frequency = <10>;
> +
> + b850v3-lvds-dp-bridge at 73  {
> + compatible = "ge,b850v3-lvds-dp";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0x73 0x72>;
> + reg-names = "main", "edid";
> +
> + interrupt-parent = <>;
> + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> + port {
> + b850v3_dp_bridge_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> +};

-- 
Regards,

Laurent Pinchart

[PATCH V7 1/4] Documentation/devicetree/bindings: b850v3_lvds_dp

[Laurent Pinchart]

Hi Peter,

On Saturday 07 Jan 2017 01:29:52 Peter Senna Tschudin wrote:
> On 04 January, 2017 21:39 CET, Rob Herring wrote:
> > On Tue, Jan 3, 2017 at 5:34 PM, Peter Senna Tschudin wrote:
> >> On 03 January, 2017 23:51 CET, Rob Herring  wrote:
> >>> On Sun, Jan 01, 2017 at 09:24:29PM +0100, Peter Senna Tschudin wrote:
>  Devicetree bindings documentation for the GE B850v3 LVDS/DP++
>  display bridge.
>  
>  Cc: Martyn Welch 
>  Cc: Martin Donnelly 
>  Cc: Javier Martinez Canillas 
>  Cc: Enric Balletbo i Serra 
>  Cc: Philipp Zabel 
>  Cc: Rob Herring 
>  Cc: Fabio Estevam 
>  Signed-off-by: Peter Senna Tschudin 
>  ---
>  There was an Acked-by from Rob Herring  for V6, but
>  I changed the bindings to use i2c_new_secondary_device() so I
>  removed it from the commit message.
>  
>   .../devicetree/bindings/ge/b850v3-lvds-dp.txt  | 39 ++
> >>> Generally, bindings are not organized by vendor. Put in
> >>> bindings/display/bridge/... instead.
> >> 
> >> Will change that.
> >> 
>   1 file changed, 39 insertions(+)
>   create mode 100644
>   Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
>  
>  diff --git a/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
>  b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt new file
>  mode 100644
>  index 000..1bc6ebf
>  --- /dev/null
>  +++ b/Documentation/devicetree/bindings/ge/b850v3-lvds-dp.txt
>  @@ -0,0 +1,39 @@
>  +Driver for GE B850v3 LVDS/DP++ display bridge
>  +
>  +Required properties:
>  +  - compatible : should be "ge,b850v3-lvds-dp".
> >>> 
> >>> Isn't '-lvds-dp' redundant? The part# should be enough.
> >> 
> >> b850v3 is the name of the product, this is why the proposed name. What
> >> about, b850v3-dp2 dp2 indicating the second DP output?
> >
> > Humm, b850v3 is the board name? This node should be the name of the bridge
> > chip.
>
> From the cover letter:
> 
> -- // --
> There are two physical bridges on the video signal pipeline: a STDP4028(LVDS
> to DP) and a STDP2690(DP to DP++).  The hardware and firmware made it
> complicated for this binding to comprise two device tree nodes, as the
> design goal is to configure both bridges based on the LVDS signal, which
> leave the driver powerless to control the video processing pipeline. The
> two bridges behaves as a single bridge, and the driver is only needed for
> telling the host about EDID / HPD, and for giving the host powers to ack
> interrupts. The video signal pipeline is as follows:
> 
>   Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
> -- // --

You forgot to prefix your patch series with [HACK] ;-)

How about fixing the issues that make the two DT nodes solution difficult ? 
What are they ?

-- 
Regards,

Laurent Pinchart

Why does OUT_FENCE_PTR point not to an fd (s32) but to an s64?

[Laurent Pinchart]

Hi Daniel,

On Monday 09 Jan 2017 11:23:23 Daniel Vetter wrote:
> On Fri, Jan 06, 2017 at 01:04:55PM -0800, Chad Versace wrote:
> > Was this a mistake in the API? If so, can we fix this ABI mistake before
> > kernel consumers rely on this?
> > 
> > I naïvely expected that OUT_FENCE_PTR would be a pointer to, obviously, a
> > fence fd (s32 __user *). But it's not. It's s64 __user *. Due to that
> > surprise, I spent several hours chasing down weird corruption in Rob
> > Clark's kmscube. The kernel unexpectedly cleared the 32 bits *above* an
> > `int kms_fence_fd` in userspace.
> 
> Never use unsized types for uabi. I guess we could have used s32, but then
> someone is going to store this in a long and it goes boom on 64 bit,

Why so ? And why do we care ? The commonly accepted practice is to store file 
descriptors in int variables. s32 is an int on all platforms, so that's fine 
too. If we use a s32 pointer here, and someone decides to store it in a long, 
bool or cast it to a complex, that's their problem :-)

> while it works on 32 bit. "int" doesn't have that problem directly, but it's
> still a red flag imo.
>
> > For reference, here's the relevant DRM code.
> > 
> > // file: drivers/gpu/drm/drm_atomic.c
> > struct drm_out_fence_state {
> > s64 __user *out_fence_ptr;
> > struct sync_file *sync_file;
> > int fd;
> > };
> > 
> > static int setup_out_fence(struct drm_out_fence_state *fence_state,
> >struct dma_fence *fence)
> > {
> > fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> > if (fence_state->fd < 0)
> > return fence_state->fd;
> > 
> > if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> > return -EFAULT;
> > 
> > fence_state->sync_file = sync_file_create(fence);
> > if (!fence_state->sync_file)
> > return -ENOMEM;
> > 
> > return 0;
> > }

-- 
Regards,

Laurent Pinchart

[Bug 97338] Black squares in the Spec Ops: The Line chapter select screen

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97338

--- Comment #7 from Samuel Pitoiset  ---
People might be interested by
https://github.com/virtual-programming/specops-linux/issues/20

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/73a0ac7a/attachment.html>

[PATCH 1/4] drm: add vblank hooks to struct drm_crtc_funcs

[Laurent Pinchart]

On Tuesday 10 Jan 2017 11:39:03 Daniel Vetter wrote:
> On Mon, Jan 09, 2017 at 07:56:24PM +0800, Shawn Guo wrote:
> > From: Shawn Guo 
> > 
> > The vblank is mostly CRTC specific and implemented as part of CRTC
> > driver.  So having vblank hooks in struct drm_crtc_funcs should
> > generally help to reduce code from client drivers in implementing
> > drm_driver's vblank callbacks.
> > 
> > Signed-off-by: Shawn Guo 
> > ---
> > 
> >  drivers/gpu/drm/drm_crtc.c | 36 
> >  include/drm/drm_crtc.h | 21 +
> >  2 files changed, 57 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 85a7452d0fb4..59ff00f48101 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -70,6 +70,42 @@ struct drm_crtc *drm_crtc_from_index(struct drm_device
> > *dev, int idx)> 
> >  EXPORT_SYMBOL(drm_crtc_from_index);
> >  
> >  /**
> > 
> > + * drm_crtc_enable_vblank - vblank enable callback helper
> > + * @dev: DRM device
> > + * @pipe: CRTC index
> > + *
> > + * It's a helper function as the generic vblank enable callback
> > implementation, + * which calls into _crtc_funcs.enable_vblank
> > function.
> > + */
> > +int drm_crtc_enable_vblank(struct drm_device *dev, unsigned int pipe)
> > +{
> > +   struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> > +
> > +   if (crtc && crtc->funcs && crtc->funcs->enable_vblank)
> > +   return crtc->funcs->enable_vblank(crtc);
> > +
> > +   return 0;
> > +}
> > +EXPORT_SYMBOL(drm_crtc_enable_vblank);
> 
> With the helper approach here there's still a pile of boilerplate in
> drivers (well, 2 lines to fill out the legacy helpers). What if instead we
> wrap all callers of enable/disable_vblank in drm_irq.c into something like
> 
> __enable_vblank(dev, pipe)
> {
>   if (DRIVER_MODESET) /* otherwise we'll oops on legacy drivers */
>   {
>   /* above code to call the new hook, if it's there. */
> 
>   if (crtc->funcs->enable_vblank)
>   return crtc->funcs->enable_vblank(crtc);
>   }
> 
>   /* fallback for everyone else */
> 
>   dev->driver->enable_vblank(dev, pipe);
> }

FWIW I like that approach much better. I'd even go as far as saying that 
DRIVER_MODESET drivers should be mass-converted.

> > +
> > +/**
> > + * drm_crtc_disable_vblank - vblank disable callback helper
> > + * @dev: DRM device
> > + * @pipe: CRTC index
> > + *
> > + * It's a helper function as the generic vblank disable callback
> > implementation,
> > + * which calls into _crtc_funcs.disable_vblank function.
> > + */
> > +void drm_crtc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> > +{
> > +   struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> > +
> > +   if (crtc && crtc->funcs && crtc->funcs->disable_vblank)
> > +   return crtc->funcs->disable_vblank(crtc);
> > +}
> > +EXPORT_SYMBOL(drm_crtc_disable_vblank);
> > +
> > +/**
> >   * drm_crtc_force_disable - Forcibly turn off a CRTC
> >   * @crtc: CRTC to turn off
> >   *

-- 
Regards,

Laurent Pinchart

[PATCH v2] drm: get fbdev size from cmdline mode if it exists

[Laurent Pinchart]

Hi Vincent,

On Tuesday 10 Jan 2017 13:33:29 Vincent ABRIOU wrote:
> On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> > On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> >> In case no connector is found while creating the fbdev, gives the
> >> possibility to specify the default fbdev size by firstly checking if the
> >> command line is defining a preferred mode. Else go into fallback and set
> >> 1024x768 fbdev size as it was already done.
> >> 
> >> Cc: Tomi Valkeinen 
> >> Signed-off-by: Vincent Abriou 
> > 
> > btw on all this there's also the possible solution to delay setup of the
> > fbdev until the first connector switches to connected, and then only
> > allocting the fb and doing the setup. Tegra has that, and Thierry did some
> > patches to move that logic into the fb helpers. But there's some locking
> > issues that need to be fixed first, hence why those patches haven't landed
> > yet.
> > 
> > But if you never probe the right mode, this here sounds like a good idea
> > too.
> 
> The early creation of fbdev is useful for userland system. If fbdev
> creation is delayed until first connector is connected, userland systems
> startup could fails (like Android that check fbdev availability at startup).
> 
> Today if no connector is connected, a default 1024x768 fbdev is created
> but it does not necessarily match the targeted CRTC size. When the
> connector is connected, fbdev is not reconfigured with the targeted CRTC
> size and it is anyway too late for the userland to take into account new
> fbdev size. Reading the cmdline is an easy way to solve this.

Isn't it another case of working around userspace issue in the kernel ? 
Shouldn't the offending userspace code be fixed ? And while at it, moved from 
fbdev to DRM/KMS ? :-) I wrote a proof-of-concept patch a few years ago to use 
DRM/KMS in the Android init process. I'm sure it doesn't apply cleanly 
anymore, but I can share it if you're interested.

-- 
Regards,

Laurent Pinchart

[Bug 99353] Kaveri 7400K shows random colored noise instead of GUI in X or Wayland

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99353

Bug ID: 99353
   Summary: Kaveri 7400K shows random colored noise instead of GUI
in X or Wayland
   Product: Mesa
   Version: 13.0
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: vedran at miletic.net
QA Contact: dri-devel at lists.freedesktop.org

I have a Kaveri 7400K on ASRock FM2A58M-VG3+. Running Fedora 25, KMS works and
it boots nicely, but instead of GDM running on Wayland, I get screen full of
colored noise. Using Xorg makes no difference.

Phoronix claims this APU worked in 2014[1]. It never worked for me with the
open source driver (previously not even KMS worked), however it worked
perfectly with fglrx last time I tried it.

I can try older Mesa and LLVM if that would be useful, but compiling it will
take a while.

[1] https://www.phoronix.com/scan.php?page=article=amd_apus_august=1

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/84ecb7ad/attachment.html>

[rfc] fix output poll execute lockdep

[Dave Airlie]

On 10 Jan. 2017 19:50, "Daniel Vetter"  wrote:

On Tue, Jan 10, 2017 at 12:12:30PM +1000, Dave Airlie wrote:
> On runtime resume, nouveau can try and take the mode_config
> mutex in the poll reenable, however a poll can race with this,
> and take the mutex and get stuck waiting for the runtime to
> finish completion. These two patches allow the driver to
> get hooked in before the mutex is taken.
>
> I think radeon/amdgpu will also need similar patches to nouveau.
>
> I found this while trying to track down a runtime regression
> on W541 laptop, I'm not sure I found what the reporter was seeing yet.

Hm, we fixed this by doing the rpm stuff always within any of the big core
locks. And I think that's the much more reasonable design, at least as
soon as you have more fine-grained locking.

But maybe there's a cheap way out - why does nouveau take the modeset
lock? Ime you can remove a lot of the kms locking super easily because
it's all cargo-culted. The last hold-out was connector_list walking, but
that's fixed now and also doesn't need any of the heavy-weight kms locks
anymore.


Reenable polling takes the lock.

Dave.


Asking since if you really need kms locks in rpm paths, and the general
design is to grab/drop rpm outside of everything (as nouveau does with the
overall ioctl wrapper trick), then there's fundamentally a deadlock, or at
least a nice inversion. It shouldn't really be needed ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/0394d537/attachment.html>

[Bug 99352] DRM/IMX first primary plane with pixel format DRM_FORMAT_YUV420 the screen is discolored

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99352

Bug ID: 99352
   Summary: DRM/IMX first primary plane with pixel format
DRM_FORMAT_YUV420 the screen is discolored
   Product: DRI
   Version: DRI git
  Hardware: ARM
OS: Linux (All)
Status: NEW
  Severity: major
  Priority: medium
 Component: DRM/other
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: zillevdr at gmx.de

Created attachment 128874
  --> https://bugs.freedesktop.org/attachment.cgi?id=128874=edit
screen picture

the fb on the first primary plane with pixel format DRM_FORMAT_YUV420 is the
screen fail colored. It could be that YUV will misinterpreted as RGB. If I copy
only y data to fb the screen is red with a shadow from the picture. Only
u-plane the screen is green and same with v-plane is blue. A picture from the
screen are attached.
Tested with kernel 4.10-rc3 and 4.9.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/df7964c2/attachment.html>

[PATCH] drm: disable deep color when EDID violates spec

[Ernst Sjöstrand]

> >> > + /* Ensure all DC modes are unset if we return early */
> >> >> > + info->edid_hdmi_dc_modes = 0;
> >> >>
> >> >> Clearing this in drm_add_display_info() should be sufficient since
> >> >> this guy doesn't get called from anywhere else. So this part could
> >> >> be droppped.
> >> >>
> >> >> Otherwise this feels like a decent way to handle the problem. We
> >> >> could of course try to use the 10bpc (or whatever) deep color modes
> >> >> the sink claims to support, but given that the people designing the
> >> >> thing didn't bother reading the spec, it seems safer to just disable
> >> >> deep color support entirely.
> >> >>
> >> >> Reviewed-by: Ville Syrjälä 
> >> >>
> >> >> > +
> >> >> >   if (cea_db_payload_len(hdmi) < 6)
> >> >> >   return;
> >> >> >
> >> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
> >> >> >   dc_bpc = 10;
> >> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
> >> >> > + modes |= DRM_EDID_HDMI_DC_30;
> >> >> >   DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
> >> >> > connector->name);
> >> >> >   }
> >> >> >
> >> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
> >> >> >   dc_bpc = 12;
> >> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
> >> >> > + modes |= DRM_EDID_HDMI_DC_36;
> >> >> >   DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
> >  >> > connector->name);
> >> >> >   }
> >> >> >
> >> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
> >> >> >   dc_bpc = 16;
> >> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
> >> >> > + modes |= DRM_EDID_HDMI_DC_48;
> >> >> >   DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
> >> >> > connector->name);
> >> >> >   }
> >> >> > @@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_
> info(struct
> >> >> > drm_connector *connector,
> >> >> >   return;
> >> >> >   }
> >> >> >
> >> >> > + /*
> >> >> > +  * All deep color modes are optional, but if a sink supports
> any
> >> >> > deep
> >> >> > +  * color mode, it must support 36-bit mode. If this is found
> not
> >> >> > +  * to be the case, sink is in violation of HDMI 1.3 / 1.4
> spec and
> >> >> > it
> >> >> > +  * is prudent to disable all deep color modes. Return here
> before
> >> >> > +  * committing bpc and edid_hdmi_dc_modes.
> >> >> > +  */
> >> >> > + if (!(modes & DRM_EDID_HDMI_DC_36)) {
> >> >> > + DRM_DEBUG("%s: HDMI sink should do DC_36, but does
> >> >> > not!\n",
> >> >> > +   connector->name);
> >> >> > + return;
> >> >> > + }
> >> >> > +
> >> >> > +
> >> >> >   DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
> >> >> > connector->name, dc_bpc);
> >> >> >   info->bpc = dc_bpc;
> >> >> > + info->edid_hdmi_dc_modes = modes;
> >> >> >
> >> >> >   /*
> >> >> >* Deep color support mandates RGB444 support for all video
> >> >> > @@ -3823,15 +3842,6 @@ static void drm_parse_hdmi_deep_color_
> info(struct
> >> >> > drm_connector *connector,
> >> >> >   DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep
> color.\n",
> >> >> > connector->name);
> >> >> >   }
> >> >> > -
> >> >> > - /*
> >> >> > -  * Spec says that if any deep color mode is supported at all,
> >> >> > -  * then deep color 36 bit must be supported.
> >> >> > -  */
> >> >> > - if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
> >> >> > - DRM_DEBUG("%s: HDMI sink should do DC_36, but does
> >> >> > not!\n",
> >> >> > -   connector->name);
> >> >> > - }
> >> >> >  }
> >> >> >
> >> >> >  static void
> >> >> > @@ -3895,6 +3905,7 @@ static void drm_add_display_info(struct
> >> >> > drm_connector *connector,
> >> >> >   /* driver figures it out in this case */
> >> >> >   info->bpc = 0;
> >> >> >   info->color_formats = 0;
> >> >> > + info->edid_hdmi_dc_modes = 0;
> >> >> >   info->cea_rev = 0;
> >> >> >   info->max_tmds_clock = 0;
> >> >> >   info->dvi_dual = false;
> >> >> > --
> >> >> > 2.11.0
> >> >>
> >> >> --
> >> >> Ville Syrjälä
> >> >> Intel OTC
> >> >> ___
> >> >> dri-devel mailing list
> >> >> dri-devel at lists.freedesktop.org
> >> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> >
> >> >
> >> >
> >> > ___
> >> > dri-devel mailing list
> >> > dri-devel at lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >> >
> >
> > --
> > Ville Syrjälä
> > Intel OTC
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/24b33a5e/attachment-0001.html>

[Bug 98984] Hexagonal shapes around lights in Cities: Skylines

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=98984

--- Comment #10 from randomsalad at gmail.com ---
(In reply to Samuel Pitoiset from comment #8)
> Thanks for confirming.
> 
> This has probably been fixed in LLVM between 3.9.1 and 4.0.0.

This I can confirm.

OpenGL renderer string: Gallium 0.4 on AMD HAWAII (DRM 3.8.0 / 4.9.0, LLVM
4.0.0)
OpenGL version string: 3.0 Mesa 17.0.0-devel (git-8bc39e251b)

Recompiled the same mesa-git as before using llvm-4.0 instead and the bug
disappeared.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/26ec2ba3/attachment-0001.html>

[PATCH] drm/i915/psr: disable psr2 for resolution greater than 32X20

[Vivi, Rodrigo]

Reviewed-by: Rodrigo Vivi 

On Tue, 2017-01-10 at 12:32 +0530, vathsala nagaraju wrote:
> PSR2 is restricted to work with panel resolutions upto 3200x2000,
> move the check to intel_psr_match_conditions and fully block psr.
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Suggested-by: Rodrigo Vivi 
> Signed-off-by: Vathsala Nagaraju 
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 15 ---
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index 6aca8ff..f2ca2a9 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -387,6 +387,13 @@ static bool intel_psr_match_conditions(struct intel_dp 
> *intel_dp)
>   return false;
>   }
>  
> + /* PSR2 is restricted to work with panel resolutions upto 3200x2000 */
> + if (intel_crtc->config->pipe_src_w > 3200 ||
> + intel_crtc->config->pipe_src_h > 2000) {
> + dev_priv->psr.psr2_support = false;
> + return false;
> + }
> +
>   dev_priv->psr.source_ok = true;
>   return true;
>  }
> @@ -425,7 +432,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
> - struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
>  
>   if (!HAS_PSR(dev_priv)) {
>   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> @@ -452,12 +458,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   hsw_psr_setup_vsc(intel_dp);
>  
>   if (dev_priv->psr.psr2_support) {
> - /* PSR2 is restricted to work with panel resolutions 
> upto 3200x2000 */
> - if (crtc->config->pipe_src_w > 3200 ||
> - crtc->config->pipe_src_h > 2000)
> - dev_priv->psr.psr2_support = false;
> - else
> - skl_psr_setup_su_vsc(intel_dp);
> + skl_psr_setup_su_vsc(intel_dp);
>   }
>  
>   /*

[PATCH] drm: disable deep color when EDID violates spec

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 12:27:45PM -0500, Alex Deucher wrote:
> On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand  wrote:
> > Isn't 10bpc very common among monitors, and 12bpc very rare? Or maybe I'm
> > confusing the transport layer with the presentation capabilities...?
> > Here are 201 monitors that claim 10bpc:
> > http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista
> >
> 
> FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
> see quite a few 10 bpc monitors.

I've seen plenty of monitors that do 10bpc over DP, but I've never seen
anything that does 10bpc over HDMI. 12bpc over HDMI is pretty common
in "proper" TVs in my experience.

> I can talk to our display team to
> see what other OSes do.

Thanks. That should give us some idea if anyone ever tried 10bpc
over HDMI on these things.

> 
> Alex
> 
> > Regards
> > //Ernst
> >
> > 2017-01-10 11:52 GMT+01:00 Ville Syrjälä  > linux.intel.com>:
> >>
> >> On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
> >> > As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
> >> > greater than 24 bits per pixel. The spec explicitly states, "All Deep
> >> > Color modes are optional though if an HDMI Source or Sink supports any
> >> > Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
> >> > Requirements).
> >> >
> >> > I came across a monitor (Acer X233H) that supplies an illegal EDID where
> >> > DC_30bit is set and DC_36bit is not set. The end result is badly garbled
> >> > output because the driver is sending 36BPP when the monitor can't handle
> >> > it.
> >> >
> >> > Much of the intel hardware is incapable of operating at any
> >> > bit-per-component setting outside of 8 or 12, and the spec seems to
> >> > imply that if any deep color support is found, then it is a safe
> >> > assumption to operate at 12.
> >> >
> >> > This patch ensures that the EDID is within the spec (specifically, that
> >> > DC_36bit is set) before committing to going forward with any deep
> >> > colors.  There was already a check for this EDID inconsistency, but it
> >> > resulted only in a warning and did not fall-back to safer settings.
> >> >
> >> > CC: Ville Syrjälä 
> >> > Signed-off-by: Nicholas Sielicki 
> >> > ---
> >> >  drivers/gpu/drm/drm_edid.c | 35 +++
> >> >  1 file changed, 23 insertions(+), 12 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> > index 336be31ff3de..42ce3f54d2dc 100644
> >> > --- a/drivers/gpu/drm/drm_edid.c
> >> > +++ b/drivers/gpu/drm/drm_edid.c
> >> > @@ -3772,30 +3772,34 @@ static void
> >> > drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
> >> >  {
> >> >   struct drm_display_info *info = >display_info;
> >> >   unsigned int dc_bpc = 0;
> >> > + u8 modes = 0;
> >> >
> >> >   /* HDMI supports at least 8 bpc */
> >> >   info->bpc = 8;
> >> >
> >> > + /* Ensure all DC modes are unset if we return early */
> >> > + info->edid_hdmi_dc_modes = 0;
> >>
> >> Clearing this in drm_add_display_info() should be sufficient since
> >> this guy doesn't get called from anywhere else. So this part could
> >> be droppped.
> >>
> >> Otherwise this feels like a decent way to handle the problem. We
> >> could of course try to use the 10bpc (or whatever) deep color modes
> >> the sink claims to support, but given that the people designing the
> >> thing didn't bother reading the spec, it seems safer to just disable
> >> deep color support entirely.
> >>
> >> Reviewed-by: Ville Syrjälä 
> >>
> >> > +
> >> >   if (cea_db_payload_len(hdmi) < 6)
> >> >   return;
> >> >
> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
> >> >   dc_bpc = 10;
> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
> >> > + modes |= DRM_EDID_HDMI_DC_30;
> >> >   DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
> >> > connector->name);
> >> >   }
> >> >
> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
> >> >   dc_bpc = 12;
> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
> >> > + modes |= DRM_EDID_HDMI_DC_36;
> >> >   DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
 >> > connector->name);
> >> >   }
> >> >
> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
> >> >   dc_bpc = 16;
> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
> >> > + modes |= DRM_EDID_HDMI_DC_48;
> >> >   DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
> >> > connector->name);
> >> >   }
> >> > @@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_info(struct
> >> > drm_connector *connector,
> >> >   return;
> >> >   }
> >> >
> >> > + /*
> >> > +  * All deep color modes are optional, but if a sink supports any
> >> > deep
> >> > +   

[Bug 192271] New: kernel 4.9 hangs during shutdown or reboot

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=192271

Bug ID: 192271
   Summary: kernel 4.9 hangs during shutdown or reboot
   Product: Drivers
   Version: 2.5
Kernel Version: 4.9.2
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: high
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-dri at kernel-bugs.osdl.org
  Reporter: upcwtenq at emltmp.com
Regression: No

After upgrading to kernel 4.9 my machine hangs during shutdown/reboot.
the shutdown/reboot procedure starts and finish with systemd unmount all
partitions.
Monitor poweroff but the gpu led (motherboard) is ON.

add "radeon.dpm=0" in kernel line solved problem.

vga: HD5770 Vapor-X (Evergreen)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 99236] System (seems to) completely freeze when interacting with java swing applications.

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99236

--- Comment #14 from Vitaly Ostrosablin  ---
85a3057f651a1c56348f1af18343d9cc0a5c93f3 used to work fine.

After that, in at most 3 commits to future from this point something was broken
and mesa didn't run (checked on 4c8c13b3568c82e503a10ddcb846b4c96261ec4c).

One of commits further in history I tried was
132b69c4edb824c70c98f8937c63e49b04f3adff, which didn't work as well.

After it, there was a huge batch of radeonsi commits.

c7dc1b010ae581f532240b661cb3d1c82e117e7e is not runnable, too.

bd56de88dfb192310f3432a3c0e0ddc3469c6d55 is runnable (probably, was fixed
somewhere earlier) and java reproducer app hangs system there.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/6f8fc808/attachment.html>

[RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 05:01:08PM +, Jose Abreu wrote:
> Hi Ville,
> 
> 
> [snip]
> 
> >> Are you going to update all the userspace clients? Exposing HDMI 2.0
> >> modes only for your favorite client doesn't sound like a good plan to
> >> me.
> >>
> >> If we simply compute from a specific modeline whether it needs to be
> >> transmitted as 4:2:0, I suppose we could simply look for a matching
> >> mode in the 4:2:0 mode. But that would mean that only the exact modes
> >> listed by the EDID will work, and others might not.
> > OK, so the 4:2:0 support is apparently listed only for specific VICs.
> 
> Hmm, the spec is not very clear. It lists video timings which may
> be used with YCbCr 4:2:0 but does not explicitly say that only
> these timings can be used. Anyway, I checked with a colleague who
> has direct access to HDMI Forum and indeed only VIC 96, 97, 101,
> 102, 106 and 107 can be used with 4:2:0, so you are correct. He
> said that the initial intention of this pixel encoding was to
> give 60Hz refresh rate in 4k to users who had limited
> performance, so it was only intended for higher resolutions.
> 
> > Hence we will need to just go through those lists to see if we can
> > (or in fact must) use 4:2:0 for a specific user specified mode.
> 
> We don't have yet support for these VICs, so this will have to
> wait :(
> 
> >
> > I would say the only slight question mark at this point is whether we
> > should favor 4:4:4 at lower bpc or 4:2:0 at higher bpc if we can choose
> > between the two. My first instinct is to favor the 4:4:4 modes for now.
> > We could add some knobs later to let the user make that choice.
> 
> I agree that 4:4:4 should be preferred.
> 
> [snip]
> 
> >>> Ok. But note that there is no nice way to figure this out. For
> >>> example, for a graphics card it all depends (apart from the
> >>> graphics HW) on the PCIe bus. If the bus is not free for enough
> >>> data rate then user can reach bottlenecks and not output at best
> >>> performance. If we gave user the ability to switch from, for
> >>> example, RGB to YCbCr 4:2:0 this bottleneck could be eliminated.
> >> Userspace won't know anything about such bottlenecks. The kernel
> >> can know it and hence should automagically drop into 4:2:0 mode
> >> if necessary.
> >>
> >>> Unless of course we always prefer YCbCr 4:2:0, when possible. I
> >>> did this internally for bridge driver dw-hdmi. We always prefer
> >>> YCbCr over RGB when they are available. It is user transparent as
> >>> the controller does the necessary color space conversion, though,
> >>> not ideal in my opinion.
> >> My idea was that we'd have a property for the output colorspace and
> >> would perhaps default to YCbCr for the CEA modes (as per CEA-861).
> >> Though I'm sure some people would cry about that behaviour as well.
> >> But for the cases where there is no choice but to use a specific
> >> output colorspace, the kernel should just do it automagically IMO. No
> >> point in manking life diffcult for userspace.
> 
> But we already have color_formats field in drm_display_info
> struct, right? Shouldn't we instead create for example a helper
> which returns the best output colorspace? According to what you
> said it would be something like:
> 
> if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
> return YCBCR444;
> else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
> return YCBCR422;
> else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR420
> && vic_is_420)
> return YCBCR420;
> else
> return RGB444; /* Mandatory by spec */

Perhaps. But it would have to be more involved than that since there
might limitations on eg. the max TMDS clock imposed by the source or
cable/dongle which presumably might require that we pick 4:2:0 over
4:4:4.

It would also need to account which formats are actually supported by
the source.

I guess for bpc it would be enough to just consider 8bpc in such a
function, and then the driver can bump it up afterwards if possible.

As for the RGB vs. YCbCr question, I guess we should just prefer RGB444
for now. And fall back to YCbCr 4:2:2 or 4:2:0 if necessary. And that 
leaves YCbCr 4:4:4 unsed since it has the same requirements as RGB
4:4:4 and thus doesn't provide any benefit as such. We could later add
a property to let the user choose between RGB vs. YCbCr more explicitly.

-- 
Ville Syrjälä
Intel OTC

[Bug 191291] [amdgpu] mouse cursor disappearing on X11 (KDE Plasma)

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=191291

--- Comment #6 from Johannes Hirte  ---
Created attachment 251151
  --> https://bugzilla.kernel.org/attachment.cgi?id=251151=edit
Xorg.0.log-4.10.0-rc3-00029-gbd5d7428f5e5

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 191291] [amdgpu] mouse cursor disappearing on X11 (KDE Plasma)

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=191291

--- Comment #5 from Johannes Hirte  ---
Created attachment 251141
  --> https://bugzilla.kernel.org/attachment.cgi?id=251141=edit
dmesg-4.10.0-rc3-00029-gbd5d7428f5e5

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 191291] [amdgpu] mouse cursor disappearing on X11 (KDE Plasma)

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=191291

--- Comment #4 from Johannes Hirte  ---
Attaching dmesg and Xorg.log from kernel 4.10.0-rc3-00029-gbd5d7428f5e5

I've booted the sytem, logged into KDE Plasma session logged out and the cursor
was gone under sddm. Logs were taken from console after logging out from
plasma.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[Bug 191281] [drm:amdgpu_ib_ring_tests] *ERROR* amdgpu: failed testing IB on ring 12 (-110)

[bugzilla-dae...@bugzilla.kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=191281

--- Comment #4 from Johannes Hirte  ---
I can confirm that
https://lists.freedesktop.org/archives/amd-gfx/2017-January/004537.html fixes
boot for me. Tested on top of linux-4.10.0-rc3-00029-gbd5d7428f5e5

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

[PATCH v2 9/9] drm/i915: Add render decompression support

[Ville Syrjälä]

On Mon, Jan 09, 2017 at 11:20:57AM -0800, Jason Ekstrand wrote:
> On Thu, Jan 5, 2017 at 7:14 AM,  wrote:
> 
> > From: Ville Syrjälä 
> >
> > SKL+ display engine can scan out certain kinds of compressed surfaces
> > produced by the render engine. This involved telling the display engine
> > the location of the color control surfae (CCS) which describes
> > which parts of the main surface are compressed and which are not. The
> > location of CCS is provided by userspace as just another plane with its
> > own offset.
> >
> > Add the required stuff to validate the user provided AUX plane metadata
> > and convert the user provided linear offset into something the hardware
> > can consume.
> >
> > Due to hardware limitations we require that the main surface and
> > the AUX surface (CCS) be part of the same bo. The hardware also
> > makes life hard by not allowing you to provide separate x/y offsets
> > for the main and AUX surfaces (excpet with NV12), so finding suitable
> > offsets for both requires a bit of work. Assuming we still want keep
> > playing tricks with the offsets. I've just gone with a dumb "search
> > backward for suitable offsets" approach, which is far from optimal,
> > but it works.
> >
> > Also not all planes will be capable of scanning out compressed surfaces,
> > and eg. 90/270 degree rotation is not supported in combination with
> > decompression either.
> >
> > This patch may contain work from at least the following people:
> > * Vandana Kannan 
> > * Daniel Vetter 
> > * Ben Widawsky 
> >
> > v2: Deal with display workarounds 0390, 0531, 1125 (Paulo)
> >
> > Cc: Paulo Zanoni 
> > Cc: Vandana Kannan 
> > Cc: Daniel Vetter 
> > Cc: Ben Widawsky 
> > Cc: Jason Ekstrand 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h  |  23 
> >  drivers/gpu/drm/i915/intel_display.c | 234 ++
> > ++---
> >  drivers/gpu/drm/i915/intel_pm.c  |  29 -
> >  drivers/gpu/drm/i915/intel_sprite.c  |   5 +
> >  4 files changed, 274 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_
> > reg.h
> > index 00970aa77afa..6849ba93f1d9 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -6209,6 +6209,28 @@ enum {
> > _ID(id, _PS_ECC_STAT_1A, _PS_ECC_STAT_2A),   \
> > _ID(id, _PS_ECC_STAT_1B, _PS_ECC_STAT_2B))
> >
> > +#define PLANE_AUX_DIST_1_A 0x701c0
> > +#define PLANE_AUX_DIST_2_A 0x702c0
> > +#define PLANE_AUX_DIST_1_B 0x711c0
> > +#define PLANE_AUX_DIST_2_B 0x712c0
> > +#define _PLANE_AUX_DIST_1(pipe) \
> > +   _PIPE(pipe, PLANE_AUX_DIST_1_A, PLANE_AUX_DIST_1_B)
> > +#define _PLANE_AUX_DIST_2(pipe) \
> > +   _PIPE(pipe, PLANE_AUX_DIST_2_A, PLANE_AUX_DIST_2_B)
> > +#define PLANE_AUX_DIST(pipe, plane) \
> > +   _MMIO_PLANE(plane, _PLANE_AUX_DIST_1(pipe),
> > _PLANE_AUX_DIST_2(pipe))
> > +
> > +#define PLANE_AUX_OFFSET_1_A   0x701c4
> > +#define PLANE_AUX_OFFSET_2_A   0x702c4
> > +#define PLANE_AUX_OFFSET_1_B   0x711c4
> > +#define PLANE_AUX_OFFSET_2_B   0x712c4
> > +#define _PLANE_AUX_OFFSET_1(pipe)   \
> > +   _PIPE(pipe, PLANE_AUX_OFFSET_1_A, PLANE_AUX_OFFSET_1_B)
> > +#define _PLANE_AUX_OFFSET_2(pipe)   \
> > +   _PIPE(pipe, PLANE_AUX_OFFSET_2_A, PLANE_AUX_OFFSET_2_B)
> > +#define PLANE_AUX_OFFSET(pipe, plane)   \
> > +   _MMIO_PLANE(plane, _PLANE_AUX_OFFSET_1(pipe),
> > _PLANE_AUX_OFFSET_2(pipe))
> > +
> >  /* legacy palette */
> >  #define _LGC_PALETTE_A   0x4a000
> >  #define _LGC_PALETTE_B   0x4a800
> > @@ -6433,6 +6455,7 @@ enum {
> >  # define CHICKEN3_DGMG_DONE_FIX_DISABLE(1 << 2)
> >
> >  #define CHICKEN_PAR1_1 _MMIO(0x42080)
> > +#define  SKL_RC_HASH_OUTSIDE   (1 << 15)
> >  #define  DPA_MASK_VBLANK_SRD   (1 << 15)
> >  #define  FORCE_ARB_IDLE_PLANES (1 << 14)
> >  #define  SKL_EDP_PSR_FIX_RDWRAP(1 << 3)
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 38de9df0ec60..2236abebd8bc 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2064,11 +2064,19 @@ intel_tile_width_bytes(const struct
> > drm_framebuffer *fb, int plane)
> > return 128;
> > else
> > return 512;
> > +   case I915_FORMAT_MOD_Y_TILED_CCS:
> > +   if (plane == 1)
> > +   return 64;
> > +   /* fall through */
> > case I915_FORMAT_MOD_Y_TILED:
> > if (IS_GEN2(dev_priv) || HAS_128_BYTE_Y_TILING(dev_priv))
> > return 128;
> > else
> > return 512;
> > +   case I915_FORMAT_MOD_Yf_TILED_CCS:
> > +   

[Bug 99350] libdrm-2.4.74/xf86drmMode.c:904:15: warning: dereferencing type-punned pointer will break strict-aliasing rules [-Wstrict-aliasing]

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99350

Bug ID: 99350
   Summary: libdrm-2.4.74/xf86drmMode.c:904:15: warning:
dereferencing type-punned pointer will break
strict-aliasing rules [-Wstrict-aliasing]
   Product: DRI
   Version: XOrg git
  Hardware: Other
   URL: https://bugs.gentoo.org/show_bug.cgi?id=605290
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: libdrm
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: mthode at mthode.org

not much to say other than it fails to work on some stricter QA checks

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/14c47a6b/attachment.html>

[Bug 99236] System (seems to) completely freeze when interacting with java swing applications.

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99236

--- Comment #13 from Ilia Mirkin  ---
Vitaly - commit id's please. Dates are largely meaningless - the default date
shown by git has little to do with when the commit made it into a particular
tree, even with mesa's rebase policy.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/1399c85d/attachment.html>

[RESEND PATCH v14 2/2] drm/i915: Put "cooked" vlank counters in frame CRC lines

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 02:43:05PM +0100, Tomeu Vizoso wrote:
> Use drm_accurate_vblank_count so we have the full 32 bit to represent
> the frame counter and userspace has a simpler way of knowing when the
> counter wraps around.
> 
> Signed-off-by: Tomeu Vizoso 
> Reviewed-by: Emil Velikov 
> Reviewed-by: Robert Foss 
> ---
> 
>  drivers/gpu/drm/i915/i915_irq.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b9beb5955dae..75fb1f66cc0c 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1557,7 +1557,6 @@ static void display_pipe_crc_irq_handler(struct 
> drm_i915_private *dev_priv,
>   struct drm_driver *driver = dev_priv->drm.driver;
>   uint32_t crcs[5];
>   int head, tail;
> - u32 frame;
>  
>   spin_lock(_crc->lock);
>   if (pipe_crc->source) {
> @@ -1612,8 +1611,9 @@ static void display_pipe_crc_irq_handler(struct 
> drm_i915_private *dev_priv,
>   crcs[2] = crc2;
>   crcs[3] = crc3;
>   crcs[4] = crc4;
> - frame = driver->get_vblank_counter(_priv->drm, pipe);
> - drm_crtc_add_crc_entry(>base, true, frame, crcs);
> + drm_crtc_add_crc_entry(>base, true,
> +drm_accurate_vblank_count(>base),

My assumption would be that this gets called after the vblank irq
handler, so using the _accurate version seems a bit overkill.

> +crcs);
>   }
>  }
>  #else
> -- 
> 2.9.3

-- 
Ville Syrjälä
Intel OTC

[RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 05:33:15PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 10, 2017 at 12:26:53PM +, Jose Abreu wrote:
> > Hi Ville,
> > 
> > 
> > On 10-01-2017 11:16, Ville Syrjälä wrote:
> > > On Thu, Jan 05, 2017 at 02:46:06PM +, Jose Abreu wrote:
> > >>
> > 
> > [snip]
> > 
> > >> The pixel clock rate is half of the TMDS character rate in 4:2:0
> > >> (in 24 bit), but for example in deep color 48 bit it will be the
> > >> same rate. There is also a reduction to half of htotal, hactive,
> > >> hblank, hfront, hsync and hback but I don't think it's a good
> > >> solution to guide us from there.
> > > I was asking if we can look at a specific modeline and whether we
> > > can tell from that if we would need to output it as 4:2:0.
> > 
> > Hmm, according to HDMI 2.0 spec there are no 4:2:0 only modes and
> > the only way to figure out if the mode is 4:2:0 only (or able) is
> > to parse the VCB and VBD blocks from EDID. The clock is half rate
> > but this is the source that has to figure it out. The mode is
> > still passed in a regular way (By VIC, by timing, ...).
> > 
> > >
> > >> Why does it feel wrong to you
> > >> expanding the uapi?
> > > Because it requires changing every single userspace kms client. And
> > > it's not something userspace should have to worry about.
> > 
> > I agree but, as Daniel said [1], we could make these new HDMI 2.0
> > features optional and only pass them to userspace if client asked
> > for them. What do you think?
> 
> Are you going to update all the userspace clients? Exposing HDMI 2.0
> modes only for your favorite client doesn't sound like a good plan to
> me.
> 
> If we simply compute from a specific modeline whether it needs to be
> transmitted as 4:2:0, I suppose we could simply look for a matching
> mode in the 4:2:0 mode. But that would mean that only the exact modes
> listed by the EDID will work, and others might not.

OK, so the 4:2:0 support is apparently listed only for specific VICs.
Hence we will need to just go through those lists to see if we can
(or in fact must) use 4:2:0 for a specific user specified mode.

I would say the only slight question mark at this point is whether we
should favor 4:4:4 at lower bpc or 4:2:0 at higher bpc if we can choose
between the two. My first instinct is to favor the 4:4:4 modes for now.
We could add some knobs later to let the user make that choice.

> 
> > 
> > [1]
> > https://lists.freedesktop.org/archives/dri-devel/2017-January/128683.html
> > 
> > >
> > >> I think its important to say that the chosen colorspace can
> > >> improve performance in systems: for example, as I said, 4:2:0
> > >> 24-bit uses half the rate that RGB 24-bit uses so we have less
> > >> trafic in the bus. I am recently working with a FPGA connected
> > >> trough pcie and I can definitely say that this is true. But, as
> > >> expected, less traffic means less quality in final image so its
> > >> not a matter of letting kernel decide, I think its a matter of
> > >> user choosing between performance vs. quality.
> > > Image quality control for userspace is a much bigger topic. And
> > > something we have no real precedent for at the moment (apart from 
> > > user choosing a different fb pixel format).
> > >
> > > The performance arument is very hardware dependent, and not really
> > > all that relevant IMO. If the user wants the big mode they either
> > > get it or not depending on whether the system can deliver.
> > >
> > 
> > Ok. But note that there is no nice way to figure this out. For
> > example, for a graphics card it all depends (apart from the
> > graphics HW) on the PCIe bus. If the bus is not free for enough
> > data rate then user can reach bottlenecks and not output at best
> > performance. If we gave user the ability to switch from, for
> > example, RGB to YCbCr 4:2:0 this bottleneck could be eliminated.
> 
> Userspace won't know anything about such bottlenecks. The kernel
> can know it and hence should automagically drop into 4:2:0 mode
> if necessary.
> 
> > Unless of course we always prefer YCbCr 4:2:0, when possible. I
> > did this internally for bridge driver dw-hdmi. We always prefer
> > YCbCr over RGB when they are available. It is user transparent as
> > the controller does the necessary color space conversion, though,
> > not ideal in my opinion.
> 
> My idea was that we'd have a property for the output colorspace and
> would perhaps default to YCbCr for the CEA modes (as per CEA-861).
> Though I'm sure some people would cry about that behaviour as well.
> But for the cases where there is no choice but to use a specific
> output colorspace, the kernel should just do it automagically IMO. No
> point in manking life diffcult for userspace.
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC

[Bug 99349] Failed to build shader (translation from TGSI)

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99349

--- Comment #1 from Enver Balalic  ---
With the R600_DEBUG=sbsafemath flag the game starts, it still spams the console
with the error. The skybox is not being rendered and an box of pink flickers on
the screen

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/f88a6815/attachment.html>

[Intel-gfx] [PATCH] drm/probe-helpers: Drop locking from poll_enable

[Daniel Vetter]

On Tue, Jan 10, 2017 at 03:17:44PM +, Chris Wilson wrote:
> On Tue, Jan 10, 2017 at 03:29:10PM +0100, Daniel Vetter wrote:
> > It was only needed to protect the connector_list walking, see
> > 
> > commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
> > Author: Daniel Vetter 
> > Date:   Thu Jul 9 23:44:26 2015 +0200
> > 
> > drm/probe-helper: Grab mode_config.mutex in poll_init/enable
> > 
> > Unfortunately the commit message of that patch fails to mention that
> > the new locking check was for the connector_list.
> > 
> > But that requirement disappeared in
> > 
> > commit c36a3254f7857f1ad9badbe3578ccc92be541a8e
> > Author: Daniel Vetter 
> > Date:   Thu Dec 15 16:58:43 2016 +0100
> > 
> > drm: Convert all helpers to drm_connector_list_iter
> > 
> > and so we can drop this again.
> > 
> > This fixes a locking inversion on nouveau, where the rpm code needs to
> > re-enable. But in other places the rpm_get() calls are nested within
> > the big modeset locks.
> > 
> > While at it, also improve the kerneldoc for these two functions a
> > notch.
> > 
> > Cc: Dave Airlie 
> > Signed-off-by: Daniel Vetter 
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c   | 41 
> > ++--
> >  drivers/gpu/drm/i915/intel_hotplug.c |  4 ++--
> >  include/drm/drm_crtc_helper.h|  1 -
> >  3 files changed, 13 insertions(+), 33 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> > b/drivers/gpu/drm/drm_probe_helper.c
> > index 060211ac74a1..7c70f2a52250 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -115,25 +115,24 @@ static int drm_helper_probe_add_cmdline_mode(struct 
> > drm_connector *connector)
> >  
> >  #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
> >  /**
> > - * drm_kms_helper_poll_enable_locked - re-enable output polling.
> > + * drm_kms_helper_poll_enable - re-enable output polling.
> >   * @dev: drm_device
> >   *
> > - * This function re-enables the output polling work without
> > - * locking the mode_config mutex.
> > + * This function re-enables the output polling work, after it has been
> > + * temporarily disabled using drm_kms_helper_poll_disable(), for example 
> > over
> > + * suspend/resume.
> >   *
> > - * This is like drm_kms_helper_poll_enable() however it is to be
> > - * called from a context where the mode_config mutex is locked
> > - * already.
> > + * Drivers can call this helper from their device resume implementation. 
> > It is
> > + * an error to call this when the output polling support has not yet been 
> > set
> > + * up.
> >   */
> > -void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
> > +void drm_kms_helper_poll_enable(struct drm_device *dev)
> >  {
> > bool poll = false;
> > struct drm_connector *connector;
> > struct drm_connector_list_iter conn_iter;
> > unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
> >  
> > -   WARN_ON(!mutex_is_locked(>mode_config.mutex));
> > -
> > if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
> > return;
> 
> Hmm, output_poll_execute() itself is not checking this correctly,
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index 84b75709af90..cb3febc6e719 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -405,7 +405,7 @@ static void output_poll_execute(struct work_struct *work)
> changed = dev->mode_config.delayed_event;
> dev->mode_config.delayed_event = false;
>  
> -   if (!drm_kms_helper_poll)
> +   if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
> goto out;

The cancel_work_sync in poll_disable should make this impossible, but it
might be a good thing to check this with a WARN_ON? Care to type that
patch as a follow up please?

> if (!mutex_trylock(>mode_config.mutex)) {
> 
> The connector list is locked, but the other reads are all racy
> (connector->polled, delayed_event). Those races seem intrinsic and handled
> by e.g. intel_hotplug.c.
> 
> Since the locking here was only covering the connector list and that now
> has its own lock,
> Reviewed-by: Chris Wilson 

Thanks for the review, I think I'll wait for Dave to supply bugzilla link
or confirmation it solves his lockdep issue before merging.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 12:26:53PM +, Jose Abreu wrote:
> Hi Ville,
> 
> 
> On 10-01-2017 11:16, Ville Syrjälä wrote:
> > On Thu, Jan 05, 2017 at 02:46:06PM +, Jose Abreu wrote:
> >>
> 
> [snip]
> 
> >> The pixel clock rate is half of the TMDS character rate in 4:2:0
> >> (in 24 bit), but for example in deep color 48 bit it will be the
> >> same rate. There is also a reduction to half of htotal, hactive,
> >> hblank, hfront, hsync and hback but I don't think it's a good
> >> solution to guide us from there.
> > I was asking if we can look at a specific modeline and whether we
> > can tell from that if we would need to output it as 4:2:0.
> 
> Hmm, according to HDMI 2.0 spec there are no 4:2:0 only modes and
> the only way to figure out if the mode is 4:2:0 only (or able) is
> to parse the VCB and VBD blocks from EDID. The clock is half rate
> but this is the source that has to figure it out. The mode is
> still passed in a regular way (By VIC, by timing, ...).
> 
> >
> >> Why does it feel wrong to you
> >> expanding the uapi?
> > Because it requires changing every single userspace kms client. And
> > it's not something userspace should have to worry about.
> 
> I agree but, as Daniel said [1], we could make these new HDMI 2.0
> features optional and only pass them to userspace if client asked
> for them. What do you think?

Are you going to update all the userspace clients? Exposing HDMI 2.0
modes only for your favorite client doesn't sound like a good plan to
me.

If we simply compute from a specific modeline whether it needs to be
transmitted as 4:2:0, I suppose we could simply look for a matching
mode in the 4:2:0 mode. But that would mean that only the exact modes
listed by the EDID will work, and others might not.

> 
> [1]
> https://lists.freedesktop.org/archives/dri-devel/2017-January/128683.html
> 
> >
> >> I think its important to say that the chosen colorspace can
> >> improve performance in systems: for example, as I said, 4:2:0
> >> 24-bit uses half the rate that RGB 24-bit uses so we have less
> >> trafic in the bus. I am recently working with a FPGA connected
> >> trough pcie and I can definitely say that this is true. But, as
> >> expected, less traffic means less quality in final image so its
> >> not a matter of letting kernel decide, I think its a matter of
> >> user choosing between performance vs. quality.
> > Image quality control for userspace is a much bigger topic. And
> > something we have no real precedent for at the moment (apart from 
> > user choosing a different fb pixel format).
> >
> > The performance arument is very hardware dependent, and not really
> > all that relevant IMO. If the user wants the big mode they either
> > get it or not depending on whether the system can deliver.
> >
> 
> Ok. But note that there is no nice way to figure this out. For
> example, for a graphics card it all depends (apart from the
> graphics HW) on the PCIe bus. If the bus is not free for enough
> data rate then user can reach bottlenecks and not output at best
> performance. If we gave user the ability to switch from, for
> example, RGB to YCbCr 4:2:0 this bottleneck could be eliminated.

Userspace won't know anything about such bottlenecks. The kernel
can know it and hence should automagically drop into 4:2:0 mode
if necessary.

> Unless of course we always prefer YCbCr 4:2:0, when possible. I
> did this internally for bridge driver dw-hdmi. We always prefer
> YCbCr over RGB when they are available. It is user transparent as
> the controller does the necessary color space conversion, though,
> not ideal in my opinion.

My idea was that we'd have a property for the output colorspace and
would perhaps default to YCbCr for the CEA modes (as per CEA-861).
Though I'm sure some people would cry about that behaviour as well.
But for the cases where there is no choice but to use a specific
output colorspace, the kernel should just do it automagically IMO. No
point in manking life diffcult for userspace.

-- 
Ville Syrjälä
Intel OTC

[Intel-gfx] [RESEND PATCH v14 2/2] drm/i915: Put "cooked" vlank counters in frame CRC lines

[Daniel Vetter]

On Tue, Jan 10, 2017 at 05:54:57PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 10, 2017 at 02:43:05PM +0100, Tomeu Vizoso wrote:
> > Use drm_accurate_vblank_count so we have the full 32 bit to represent
> > the frame counter and userspace has a simpler way of knowing when the
> > counter wraps around.
> > 
> > Signed-off-by: Tomeu Vizoso 
> > Reviewed-by: Emil Velikov 
> > Reviewed-by: Robert Foss 
> > ---
> > 
> >  drivers/gpu/drm/i915/i915_irq.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c 
> > b/drivers/gpu/drm/i915/i915_irq.c
> > index b9beb5955dae..75fb1f66cc0c 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1557,7 +1557,6 @@ static void display_pipe_crc_irq_handler(struct 
> > drm_i915_private *dev_priv,
> > struct drm_driver *driver = dev_priv->drm.driver;
> > uint32_t crcs[5];
> > int head, tail;
> > -   u32 frame;
> >  
> > spin_lock(_crc->lock);
> > if (pipe_crc->source) {
> > @@ -1612,8 +1611,9 @@ static void display_pipe_crc_irq_handler(struct 
> > drm_i915_private *dev_priv,
> > crcs[2] = crc2;
> > crcs[3] = crc3;
> > crcs[4] = crc4;
> > -   frame = driver->get_vblank_counter(_priv->drm, pipe);
> > -   drm_crtc_add_crc_entry(>base, true, frame, crcs);
> > +   drm_crtc_add_crc_entry(>base, true,
> > +  drm_accurate_vblank_count(>base),
> 
> My assumption would be that this gets called after the vblank irq
> handler, so using the _accurate version seems a bit overkill.

Since we're at like v15 of this I figured I'll pull this in, and we can
polish this a bit more later. Tomeu, can you pls do that follow-up patch
and get Ville to review+merge it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[Bug 99349] Failed to build shader (translation from TGSI)

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99349

Bug ID: 99349
   Summary: Failed to build shader (translation from TGSI)
   Product: Mesa
   Version: 13.0
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/r600
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: balalic.enver at gmail.com
QA Contact: dri-devel at lists.freedesktop.org

Created attachment 128864
  --> https://bugs.freedesktop.org/attachment.cgi?id=128864=edit
glxinfo

I'm using a Radeon HD6950, it fails to build a shader when playing "War
Thunder", renders the login screen fine, starts to build shaders and then fails
and spams the following in the console:

EE r600_state_common.c:799 r600_shader_select - Failed to build shader variant
(type=1) -1
EE r600_shader.c:183 r600_pipe_shader_create - translation from TGSI failed !

same thing happens a lof of shaders from shadertoy.com.

OS: OpenSUSE Tumbleweed, glxinfo attached

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/2d119a10/attachment.html>

[Bug 99236] System (seems to) completely freeze when interacting with java swing applications.

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99236

--- Comment #12 from Vitaly Ostrosablin  ---
Looks like it broke on Dec 07. There was a lot of radeonsi-related commits, but
I had difficulty compiling a working mesa out of them. On Dec 6, there was no
bug. No commits on Dec 7 seems to work, they're segfaulting. Then later on Dec
8, mesa can be compiled an started, but issue is already present.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/0801e0c6/attachment.html>

[bug report] drm: kselftest for drm_mm and eviction

[Dan Carpenter]

Hello Chris Wilson,

The patch 560b32842912: "drm: kselftest for drm_mm and eviction" from
Dec 22, 2016, leads to the following static checker warning:

drivers/gpu/drm/selftests/test-drm_mm.c:1277 evict_everything()
warn: calling list_del() inside list_for_each

drivers/gpu/drm/selftests/test-drm_mm.c
  1260  static bool evict_everything(struct drm_mm *mm,
  1261   unsigned int total_size,
  1262   struct evict_node *nodes)
  1263  {
  1264  struct drm_mm_scan scan;
  1265  LIST_HEAD(evict_list);
  1266  struct evict_node *e;
  1267  unsigned int n;
  1268  int err;
  1269  
  1270  drm_mm_scan_init(, mm, total_size, 0, 0, 0);
  1271  for (n = 0; n < total_size; n++) {
  1272  e = [n];
  1273  list_add(>link, _list);
  1274  if (drm_mm_scan_add_block(, >node))
  1275  break;
  1276  }
  1277  list_for_each_entry(e, _list, link) {
  1278  if (!drm_mm_scan_remove_block(, >node)) {
  1279  pr_err("Node %lld not marked for eviction!\n",
  1280 e->node.start);
  1281  list_del(>link);

Probably this should be using list_for_each_entry_safe(), I guess?

  1282  }
  1283  }
  1284  
  1285  list_for_each_entry(e, _list, link)
  1286  drm_mm_remove_node(>node);
  1287  
  1288  if (!assert_one_hole(mm, 0, total_size))
  1289  return false;
  1290  
  1291  list_for_each_entry(e, _list, link) {
  1292  err = drm_mm_reserve_node(mm, >node);
  1293  if (err) {
  1294  pr_err("Failed to reinsert node after eviction: 
start=%llx\n",
  1295 e->node.start);
  1296  return false;
  1297  }
  1298  }
  1299  
  1300  return assert_continuous(mm, nodes[0].node.size);
  1301  }

regards,
dan carpenter

[Intel-gfx] [PATCH] drm/probe-helpers: Drop locking from poll_enable

[Chris Wilson]

On Tue, Jan 10, 2017 at 05:34:08PM +0100, Daniel Vetter wrote:
> On Tue, Jan 10, 2017 at 03:17:44PM +, Chris Wilson wrote:
> > On Tue, Jan 10, 2017 at 03:29:10PM +0100, Daniel Vetter wrote:
> > > -void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
> > > +void drm_kms_helper_poll_enable(struct drm_device *dev)
> > >  {
> > >   bool poll = false;
> > >   struct drm_connector *connector;
> > >   struct drm_connector_list_iter conn_iter;
> > >   unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
> > >  
> > > - WARN_ON(!mutex_is_locked(>mode_config.mutex));
> > > -
> > >   if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
> > >   return;
> > 
> > Hmm, output_poll_execute() itself is not checking this correctly,
> > 
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> > b/drivers/gpu/drm/drm_probe_helper.c
> > index 84b75709af90..cb3febc6e719 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -405,7 +405,7 @@ static void output_poll_execute(struct work_struct 
> > *work)
> > changed = dev->mode_config.delayed_event;
> > dev->mode_config.delayed_event = false;
> >  
> > -   if (!drm_kms_helper_poll)
> > +   if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
> > goto out;
> 
> The cancel_work_sync in poll_disable should make this impossible, but it
> might be a good thing to check this with a WARN_ON? Care to type that
> patch as a follow up please?

Imagine a drm_kms_helper_poll_disable() run concurrently with
drm_kms_helper_poll_enable(). The enable() gets past the is-enabled? check
and begins iterating the list, meanwhile the disable() cancels the work
and turns off the helper. The enable() is unaware of this and so
reschedules the work, and as output_poll_execute() doesn't check for
dev->mode_config.poll_enabled it stays active.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[Bug 97879] [amdgpu] Rocket League: long hangs (several seconds) when loading assets (models/textures/shaders?)

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=97879

--- Comment #50 from Marek Olšák  ---
It would be useful to have a debug build of the game and run it with a profiler
or debugger to see where it's looping.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/aa2c00ed/attachment.html>

[RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB

[Jose Abreu]

Hi Ville,


[snip]

>> Are you going to update all the userspace clients? Exposing HDMI 2.0
>> modes only for your favorite client doesn't sound like a good plan to
>> me.
>>
>> If we simply compute from a specific modeline whether it needs to be
>> transmitted as 4:2:0, I suppose we could simply look for a matching
>> mode in the 4:2:0 mode. But that would mean that only the exact modes
>> listed by the EDID will work, and others might not.
> OK, so the 4:2:0 support is apparently listed only for specific VICs.

Hmm, the spec is not very clear. It lists video timings which may
be used with YCbCr 4:2:0 but does not explicitly say that only
these timings can be used. Anyway, I checked with a colleague who
has direct access to HDMI Forum and indeed only VIC 96, 97, 101,
102, 106 and 107 can be used with 4:2:0, so you are correct. He
said that the initial intention of this pixel encoding was to
give 60Hz refresh rate in 4k to users who had limited
performance, so it was only intended for higher resolutions.

> Hence we will need to just go through those lists to see if we can
> (or in fact must) use 4:2:0 for a specific user specified mode.

We don't have yet support for these VICs, so this will have to
wait :(

>
> I would say the only slight question mark at this point is whether we
> should favor 4:4:4 at lower bpc or 4:2:0 at higher bpc if we can choose
> between the two. My first instinct is to favor the 4:4:4 modes for now.
> We could add some knobs later to let the user make that choice.

I agree that 4:4:4 should be preferred.

[snip]

>>> Ok. But note that there is no nice way to figure this out. For
>>> example, for a graphics card it all depends (apart from the
>>> graphics HW) on the PCIe bus. If the bus is not free for enough
>>> data rate then user can reach bottlenecks and not output at best
>>> performance. If we gave user the ability to switch from, for
>>> example, RGB to YCbCr 4:2:0 this bottleneck could be eliminated.
>> Userspace won't know anything about such bottlenecks. The kernel
>> can know it and hence should automagically drop into 4:2:0 mode
>> if necessary.
>>
>>> Unless of course we always prefer YCbCr 4:2:0, when possible. I
>>> did this internally for bridge driver dw-hdmi. We always prefer
>>> YCbCr over RGB when they are available. It is user transparent as
>>> the controller does the necessary color space conversion, though,
>>> not ideal in my opinion.
>> My idea was that we'd have a property for the output colorspace and
>> would perhaps default to YCbCr for the CEA modes (as per CEA-861).
>> Though I'm sure some people would cry about that behaviour as well.
>> But for the cases where there is no choice but to use a specific
>> output colorspace, the kernel should just do it automagically IMO. No
>> point in manking life diffcult for userspace.

But we already have color_formats field in drm_display_info
struct, right? Shouldn't we instead create for example a helper
which returns the best output colorspace? According to what you
said it would be something like:

if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR444)
return YCBCR444;
else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR422)
return YCBCR422;
else if (display_info->color_formats & DRM_COLOR_FORMAT_YCBCR420
&& vic_is_420)
return YCBCR420;
else
return RGB444; /* Mandatory by spec */

>>
>> -- 
>> Ville Syrjälä
>> Intel OTC

Best regards,
Jose Miguel Abreu

[Bug 99330] Severe flickering with Fiji on Wayland

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99330

--- Comment #6 from Vedran Miletić  ---
Got it, thanks Ernst:

Jan 10 17:44:30 localhost.localdomain gnome-shell[22110]: Failed to flip:
Invalid argument

I also got the following in dmesg, but it's likely unrelated as I get it with
Xorg as well:

[Tue Jan 10 17:44:29 2017] [drm:amdgpu_crtc_page_flip [amdgpu]] *ERROR* failed
to get vblank before flip

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/05406261/attachment.html>

[PATCH] drm: disable deep color when EDID violates spec

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 03:39:42PM +0200, Jani Nikula wrote:
> On Tue, 10 Jan 2017, Ville Syrjälä  wrote:
> > On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
> >> As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
> >> greater than 24 bits per pixel. The spec explicitly states, "All Deep
> >> Color modes are optional though if an HDMI Source or Sink supports any
> >> Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
> >> Requirements).
> >> 
> >> I came across a monitor (Acer X233H) that supplies an illegal EDID where
> >> DC_30bit is set and DC_36bit is not set. The end result is badly garbled
> >> output because the driver is sending 36BPP when the monitor can't handle
> >> it.
> >> 
> >> Much of the intel hardware is incapable of operating at any
> >> bit-per-component setting outside of 8 or 12, and the spec seems to
> >> imply that if any deep color support is found, then it is a safe
> >> assumption to operate at 12.
> >> 
> >> This patch ensures that the EDID is within the spec (specifically, that
> >> DC_36bit is set) before committing to going forward with any deep
> >> colors.  There was already a check for this EDID inconsistency, but it
> >> resulted only in a warning and did not fall-back to safer settings.
> >>
> 
> I thought there was a related bugzilla? Where's the reference?

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99250

> 
> >> CC: Ville Syrjälä 
> >> Signed-off-by: Nicholas Sielicki 
> >> ---
> >>  drivers/gpu/drm/drm_edid.c | 35 +++
> >>  1 file changed, 23 insertions(+), 12 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >> index 336be31ff3de..42ce3f54d2dc 100644
> >> --- a/drivers/gpu/drm/drm_edid.c
> >> +++ b/drivers/gpu/drm/drm_edid.c
> >> @@ -3772,30 +3772,34 @@ static void drm_parse_hdmi_deep_color_info(struct 
> >> drm_connector *connector,
> >>  {
> >>struct drm_display_info *info = >display_info;
> >>unsigned int dc_bpc = 0;
> >> +  u8 modes = 0;
> >>  
> >>/* HDMI supports at least 8 bpc */
> >>info->bpc = 8;
> >>  
> >> +  /* Ensure all DC modes are unset if we return early */
> >> +  info->edid_hdmi_dc_modes = 0;
> >
> > Clearing this in drm_add_display_info() should be sufficient since
> > this guy doesn't get called from anywhere else. So this part could
> > be droppped.
> >
> > Otherwise this feels like a decent way to handle the problem. We
> > could of course try to use the 10bpc (or whatever) deep color modes
> > the sink claims to support, but given that the people designing the
> > thing didn't bother reading the spec, it seems safer to just disable
> > deep color support entirely.
> 
> Hmmh.
> 
> So currently, the sink in question violates this, "All Deep Color modes
> are optional though if an HDMI Source or Sink supports any Deep Color
> mode, it shall support 36-bit mode."
> 
> But also currently, we violate this, "An HDMI Source shall not send any
> Deep Color mode to a Sink that does not indicate support for that mode."
> 
> Instead of fixing our violation, this patch points fingers at the
> violating sinks, and refuses to play ball with any deep colors.
> 
> Just to get the record straight, is that a fair assesment?

More or less. i915 can't violate the spec unless the sink violates the
spec as well. I did actually write a patch once to explicitly check the
DC_36 bit in i915 code, but I don't think I ever sent it out as I
figured it's an impossible scenario. But I should have known better and
assumed that some sink will eventually violate the spec.

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm: disable deep color when EDID violates spec

[Harry Wentland]

On 2017-01-10 03:41 PM, Harry Wentland wrote:
> On 2017-01-10 03:10 PM, Alex Deucher wrote:
>> On Tue, Jan 10, 2017 at 3:02 PM, Ernst Sjöstrand 
>> wrote:
>>> I kindof assume DP is the default connection these days and with
>>> Freesync
>>> you use
>>> DP or course, but this question was specifically for HDMI.
>>> I guess this patch doesn't affect deep color over DP?
>>>
>>> Anyway, only 17 of those monitors have FreeSync but almost all have
>>> DP, so
>>> perhaps they only support
>>> 10 bpc when connected with DP?
>>
>> We see 10 bpc only over HDMI a lot apparently.  I guess a lot of
>> monitor vendors do the minimum necessary for deep color.
>>
>
> Yes, apparently there are a bunch of HDMI displays that we drive in
> 10bpc that might or might not report 12bpc support. From talking to our
> HDMI guys it sounds like this is a slightly ambiguous area in the spec,
> despite what the HDMI spec quote mentioned by Nicholas said. I'll see if
> I can get more info.
>

Adding Ernst, Nicholas, Ville, Alex again.

So apparently the spec is pretty clear and there's a sink compliance 
test that should cover this. I don't really think it makes sense for the 
source device to babysit the sink's behavior, though.

In general we might want to try for a solution that gives the best user 
experience and highest interoperability, so check what sink supports, 
check what source supports, check what cable can do, and do an 
intersection of all to give the best user experience.

I suggest to block 10-bit on driver's that can't handle this.

Harry


> I'm not sure it makes sense to block all deep color modes in this case
> for all drivers. Other HW (e.g. AMD's) is perfectly capable of driving
> 10-bit. Would it make sense to just block this on the i915 side as Ville
> alluded to on another thread?
>
> Harry
>
>> Alex
>>
>>>
>>> Regards
>>> //Ernst
>>>
>>> 2017-01-10 20:54 GMT+01:00 Alex Deucher :

 On Tue, Jan 10, 2017 at 12:46 PM, Ville Syrjälä
  wrote:
> On Tue, Jan 10, 2017 at 12:27:45PM -0500, Alex Deucher wrote:
>> On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand 
>> wrote:
>>> Isn't 10bpc very common among monitors, and 12bpc very rare? Or
>>> maybe
>>> I'm
>>> confusing the transport layer with the presentation capabilities...?
>>> Here are 201 monitors that claim 10bpc:
>>> http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista
>>>
>>
>> FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
>> see quite a few 10 bpc monitors.
>
> I've seen plenty of monitors that do 10bpc over DP, but I've never
> seen
> anything that does 10bpc over HDMI. 12bpc over HDMI is pretty common
> in "proper" TVs in my experience.
>
>> I can talk to our display team to
>> see what other OSes do.
>
> Thanks. That should give us some idea if anyone ever tried 10bpc
> over HDMI on these things.

 We apparently see this pretty commonly, especially with freesync
 monitors, and we support it.  It seems to be pretty prevalent because
 you can support a higher refresh rate with a lower bpc.

 Alex

>
>>
>> Alex
>>
>>> Regards
>>> //Ernst
>>>
>>> 2017-01-10 11:52 GMT+01:00 Ville Syrjälä
>>> :

 On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
> As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color
> depths
> greater than 24 bits per pixel. The spec explicitly states, "All
> Deep
> Color modes are optional though if an HDMI Source or Sink supports
> any
> Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
> Requirements).
>
> I came across a monitor (Acer X233H) that supplies an illegal EDID
> where
> DC_30bit is set and DC_36bit is not set. The end result is badly
> garbled
> output because the driver is sending 36BPP when the monitor can't
> handle
> it.
>
> Much of the intel hardware is incapable of operating at any
> bit-per-component setting outside of 8 or 12, and the spec seems
> to
> imply that if any deep color support is found, then it is a safe
> assumption to operate at 12.
>
> This patch ensures that the EDID is within the spec (specifically,
> that
> DC_36bit is set) before committing to going forward with any deep
> colors.  There was already a check for this EDID inconsistency,
> but it
> resulted only in a warning and did not fall-back to safer
> settings.
>
> CC: Ville Syrjälä 
> Signed-off-by: Nicholas Sielicki 
> ---
>  drivers/gpu/drm/drm_edid.c | 35
> +++
>  1 file changed, 23 insertions(+), 12 deletions(-)
>
> diff --git 

[PATCH 03/10] drm/i915/psr: fix blank screen issue for psr2

[vathsala nagaraju]

On Monday 09 January 2017 10:47 PM, Vivi, Rodrigo wrote:
> On Mon, 2017-01-09 at 18:26 +0530, vathsala nagaraju wrote:
>> Psr1 and psr2 are mutually exclusive,ie when psr2 is enabled,
>> psr1 should be disabled.When psr2 is exited , bit 31 of reg
>> PSR2_CTL must be set to 0 but currently bit 31 of SRD_CTL
>> (psr1 control register)is set to 0.
>> Also ,PSR2_IDLE state is looked up from SRD_STATUS(psr1 register)
>> instead of PSR2_STATUS register, which has wrong data, resulting
>> in blankscreen.
>> hsw_enable_source is split into hsw_enable_source_psr1 and
>> hsw_enable_source_psr2 for easier code review and maintenance,
>> as suggested by rodrigo and jim.
>>
>> v2: (Rodrigo)
>> - Rename hsw_enable_source_psr* to intel_enable_source_psr*
>>
>> v3: (Rodrigo)
>> - In hsw_psr_disable ,
>>1) for psr active case, handle psr2 followed by psr1.
>>2) psr inactive case, handle psr2 followed by psr1
> much better, thanks, but still has one blocking issue imho:
>
>> Cc: Rodrigo Vivi 
>> Cc: Jim Bride 
>> Signed-off-by: Vathsala Nagaraju 
>> Signed-off-by: Patil Deepti 
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h  |   3 +
>>   drivers/gpu/drm/i915/intel_psr.c | 126 
>> +--
>>   2 files changed, 97 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 00970aa..7830e6e 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3615,6 +3615,9 @@ enum {
>>   #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
>>   #define   EDP_PSR2_IDLE_MASK   0xf
>>   
>> +#define EDP_PSR2_STATUS_CTL_MMIO(0x6f940)
>> +#define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
>> +
>>   /* VGA port control */
>>   #define ADPA   _MMIO(0x61100)
>>   #define PCH_ADPA_MMIO(0xe1100)
>> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
>> b/drivers/gpu/drm/i915/intel_psr.c
>> index c3aa649..6c161aa 100644
>> --- a/drivers/gpu/drm/i915/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/intel_psr.c
>> @@ -261,12 +261,11 @@ static void vlv_psr_activate(struct intel_dp *intel_dp)
>> VLV_EDP_PSR_ACTIVE_ENTRY);
>>   }
>>   
>> -static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>> +static void intel_enable_source_psr1(struct intel_dp *intel_dp)
>>   {
>>  struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>>  struct drm_device *dev = dig_port->base.base.dev;
>>  struct drm_i915_private *dev_priv = to_i915(dev);
>> -
>>  uint32_t max_sleep_time = 0x1f;
>>  /*
>>   * Let's respect VBT in case VBT asks a higher idle_frame value.
>> @@ -312,14 +311,30 @@ static void hsw_psr_enable_source(struct intel_dp 
>> *intel_dp)
>>  val |= EDP_PSR_TP1_TP2_SEL;
>>   
>>  I915_WRITE(EDP_PSR_CTL, val);
>> +}
>>   
>> -if (!dev_priv->psr.psr2_support)
>> -return;
>> +static void intel_enable_source_psr2(struct intel_dp *intel_dp)
>> +{
>> +struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> +struct drm_device *dev = dig_port->base.base.dev;
>> +struct drm_i915_private *dev_priv = to_i915(dev);
>> +
>> +/*
>> + * Let's respect VBT in case VBT asks a higher idle_frame value.
>> + * Let's use 6 as the minimum to cover all known cases including
>> + * the off-by-one issue that HW has in some cases. Also there are
>> + * cases where sink should be able to train
>> + * with the 5 or 6 idle patterns.
>> + */
>> +uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
>> +uint32_t val = EDP_PSR_ENABLE;
>> +
>> +val |= idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
>>   
>>  /* FIXME: selective update is probably totally broken because it doesn't
>>   * mesh at all with our frontbuffer tracking. And the hw alone isn't
>>   * good enough. */
>> -val = EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
>> +val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
>>   
>>  if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
>>  val |= EDP_PSR2_TP2_TIME_2500;
>> @@ -333,6 +348,20 @@ static void hsw_psr_enable_source(struct intel_dp 
>> *intel_dp)
>>  I915_WRITE(EDP_PSR2_CTL, val);
>>   }
>>   
>> +
>> +static void hsw_psr_enable_source(struct intel_dp *intel_dp)
>> +{
>> +struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> +struct drm_device *dev = dig_port->base.base.dev;
>> +struct drm_i915_private *dev_priv = to_i915(dev);
>> +
>> +/* psr1 and psr2 are mutually exclusive.*/
>> +if (dev_priv->psr.psr2_support)
>> +intel_enable_source_psr2(intel_dp);
>> +else
>> +intel_enable_source_psr1(intel_dp);
>> +}
>> +
>>   static bool intel_psr_match_conditions(struct intel_dp *intel_dp)
>>   {
>>  struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
>> @@ -410,7 +439,10 @@ static void intel_psr_activate(struct intel_dp 
>> *intel_dp)
>>  

[PULL] drm-qemu: 4.10 updates

[Gerd Hoffmann]

  Hi,

Sorry, I'm a little late for the 4.10 merge window (too busy with other
stuff in december).  The only code change in this pull is a one-line
sparse fix for virtio-gpu though, the other changes are docs only.  So I
think it should be fine for 4.10 at this point.  If you disagree feel
free to queue up for 4.11 instead.

thanks,
  Gerd

The following changes since commit
a121103c922847ba5010819a3f250f1f7fc84ab8:

  Linux 4.10-rc3 (2017-01-08 14:18:17 -0800)

are available in the git repository at:

  git://git.kraxel.org/linux tags/drm-qemu-20170110

for you to fetch changes up to af3076e67c31ceb3e314933dd61cb68a1d5120cf:

  drm: flip cirrus driver status to "obsolete". (2017-01-10 14:00:40
+0100)


drm-qemu: virtio sparse fix, MAINTAINERS updates.


Gerd Hoffmann (3):
  drm/virtio: fix framebuffer sparse warning
  drm: update MAINTAINERS for qemu drivers (bochs, cirrus, qxl,
virtio-gpu)
  drm: flip cirrus driver status to "obsolete".

 MAINTAINERS | 16 +---
 drivers/gpu/drm/cirrus/Kconfig  |  9 +
 drivers/gpu/drm/virtio/virtgpu_fb.c |  2 +-
 3 files changed, 23 insertions(+), 4 deletions(-)

[PATCH] drm: disable deep color when EDID violates spec

[Harry Wentland]

On 2017-01-10 03:10 PM, Alex Deucher wrote:
> On Tue, Jan 10, 2017 at 3:02 PM, Ernst Sjöstrand  wrote:
>> I kindof assume DP is the default connection these days and with Freesync
>> you use
>> DP or course, but this question was specifically for HDMI.
>> I guess this patch doesn't affect deep color over DP?
>>
>> Anyway, only 17 of those monitors have FreeSync but almost all have DP, so
>> perhaps they only support
>> 10 bpc when connected with DP?
>
> We see 10 bpc only over HDMI a lot apparently.  I guess a lot of
> monitor vendors do the minimum necessary for deep color.
>

Yes, apparently there are a bunch of HDMI displays that we drive in 
10bpc that might or might not report 12bpc support. From talking to our 
HDMI guys it sounds like this is a slightly ambiguous area in the spec, 
despite what the HDMI spec quote mentioned by Nicholas said. I'll see if 
I can get more info.

I'm not sure it makes sense to block all deep color modes in this case 
for all drivers. Other HW (e.g. AMD's) is perfectly capable of driving 
10-bit. Would it make sense to just block this on the i915 side as Ville 
alluded to on another thread?

Harry

> Alex
>
>>
>> Regards
>> //Ernst
>>
>> 2017-01-10 20:54 GMT+01:00 Alex Deucher :
>>>
>>> On Tue, Jan 10, 2017 at 12:46 PM, Ville Syrjälä
>>>  wrote:
 On Tue, Jan 10, 2017 at 12:27:45PM -0500, Alex Deucher wrote:
> On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand 
> wrote:
>> Isn't 10bpc very common among monitors, and 12bpc very rare? Or maybe
>> I'm
>> confusing the transport layer with the presentation capabilities...?
>> Here are 201 monitors that claim 10bpc:
>> http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista
>>
>
> FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
> see quite a few 10 bpc monitors.

 I've seen plenty of monitors that do 10bpc over DP, but I've never seen
 anything that does 10bpc over HDMI. 12bpc over HDMI is pretty common
 in "proper" TVs in my experience.

> I can talk to our display team to
> see what other OSes do.

 Thanks. That should give us some idea if anyone ever tried 10bpc
 over HDMI on these things.
>>>
>>> We apparently see this pretty commonly, especially with freesync
>>> monitors, and we support it.  It seems to be pretty prevalent because
>>> you can support a higher refresh rate with a lower bpc.
>>>
>>> Alex
>>>

>
> Alex
>
>> Regards
>> //Ernst
>>
>> 2017-01-10 11:52 GMT+01:00 Ville Syrjälä
>> :
>>>
>>> On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
 As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color
 depths
 greater than 24 bits per pixel. The spec explicitly states, "All
 Deep
 Color modes are optional though if an HDMI Source or Sink supports
 any
 Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
 Requirements).

 I came across a monitor (Acer X233H) that supplies an illegal EDID
 where
 DC_30bit is set and DC_36bit is not set. The end result is badly
 garbled
 output because the driver is sending 36BPP when the monitor can't
 handle
 it.

 Much of the intel hardware is incapable of operating at any
 bit-per-component setting outside of 8 or 12, and the spec seems
 to
 imply that if any deep color support is found, then it is a safe
 assumption to operate at 12.

 This patch ensures that the EDID is within the spec (specifically,
 that
 DC_36bit is set) before committing to going forward with any deep
 colors.  There was already a check for this EDID inconsistency,
 but it
 resulted only in a warning and did not fall-back to safer
 settings.

 CC: Ville Syrjälä 
 Signed-off-by: Nicholas Sielicki 
 ---
  drivers/gpu/drm/drm_edid.c | 35
 +++
  1 file changed, 23 insertions(+), 12 deletions(-)

 diff --git a/drivers/gpu/drm/drm_edid.c
 b/drivers/gpu/drm/drm_edid.c
 index 336be31ff3de..42ce3f54d2dc 100644
 --- a/drivers/gpu/drm/drm_edid.c
 +++ b/drivers/gpu/drm/drm_edid.c
 @@ -3772,30 +3772,34 @@ static void
 drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
  {
   struct drm_display_info *info = >display_info;
   unsigned int dc_bpc = 0;
 + u8 modes = 0;

   /* HDMI supports at least 8 bpc */
   info->bpc = 8;

 + /* Ensure all DC modes are unset if we return early */
 + info->edid_hdmi_dc_modes = 0;
>>>
>>> Clearing this in drm_add_display_info() should be sufficient since
>>> this guy doesn't 

[PATCH] drm: disable deep color when EDID violates spec

[Jani Nikula]

On Tue, 10 Jan 2017, Ville Syrjälä  wrote:
> On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
>> As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
>> greater than 24 bits per pixel. The spec explicitly states, "All Deep
>> Color modes are optional though if an HDMI Source or Sink supports any
>> Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
>> Requirements).
>> 
>> I came across a monitor (Acer X233H) that supplies an illegal EDID where
>> DC_30bit is set and DC_36bit is not set. The end result is badly garbled
>> output because the driver is sending 36BPP when the monitor can't handle
>> it.
>> 
>> Much of the intel hardware is incapable of operating at any
>> bit-per-component setting outside of 8 or 12, and the spec seems to
>> imply that if any deep color support is found, then it is a safe
>> assumption to operate at 12.
>> 
>> This patch ensures that the EDID is within the spec (specifically, that
>> DC_36bit is set) before committing to going forward with any deep
>> colors.  There was already a check for this EDID inconsistency, but it
>> resulted only in a warning and did not fall-back to safer settings.
>>

I thought there was a related bugzilla? Where's the reference?

>> CC: Ville Syrjälä 
>> Signed-off-by: Nicholas Sielicki 
>> ---
>>  drivers/gpu/drm/drm_edid.c | 35 +++
>>  1 file changed, 23 insertions(+), 12 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> index 336be31ff3de..42ce3f54d2dc 100644
>> --- a/drivers/gpu/drm/drm_edid.c
>> +++ b/drivers/gpu/drm/drm_edid.c
>> @@ -3772,30 +3772,34 @@ static void drm_parse_hdmi_deep_color_info(struct 
>> drm_connector *connector,
>>  {
>>  struct drm_display_info *info = >display_info;
>>  unsigned int dc_bpc = 0;
>> +u8 modes = 0;
>>  
>>  /* HDMI supports at least 8 bpc */
>>  info->bpc = 8;
>>  
>> +/* Ensure all DC modes are unset if we return early */
>> +info->edid_hdmi_dc_modes = 0;
>
> Clearing this in drm_add_display_info() should be sufficient since
> this guy doesn't get called from anywhere else. So this part could
> be droppped.
>
> Otherwise this feels like a decent way to handle the problem. We
> could of course try to use the 10bpc (or whatever) deep color modes
> the sink claims to support, but given that the people designing the
> thing didn't bother reading the spec, it seems safer to just disable
> deep color support entirely.

Hmmh.

So currently, the sink in question violates this, "All Deep Color modes
are optional though if an HDMI Source or Sink supports any Deep Color
mode, it shall support 36-bit mode."

But also currently, we violate this, "An HDMI Source shall not send any
Deep Color mode to a Sink that does not indicate support for that mode."

Instead of fixing our violation, this patch points fingers at the
violating sinks, and refuses to play ball with any deep colors.

Just to get the record straight, is that a fair assesment?

BR,
Jani.


>
> Reviewed-by: Ville Syrjälä 
>
>> +
>>  if (cea_db_payload_len(hdmi) < 6)
>>  return;
>>  
>>  if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
>>  dc_bpc = 10;
>> -info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
>> +modes |= DRM_EDID_HDMI_DC_30;
>>  DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
>>connector->name);
>>  }
>>  
>>  if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
>>  dc_bpc = 12;
>> -info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
>> +modes |= DRM_EDID_HDMI_DC_36;
>>  DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
>>connector->name);
>>  }
>>  
>>  if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
>>  dc_bpc = 16;
>> -info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
>> +modes |= DRM_EDID_HDMI_DC_48;
>>  DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
>>connector->name);
>>  }
>> @@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_info(struct 
>> drm_connector *connector,
>>  return;
>>  }
>>  
>> +/*
>> + * All deep color modes are optional, but if a sink supports any deep
>> + * color mode, it must support 36-bit mode. If this is found not
>> + * to be the case, sink is in violation of HDMI 1.3 / 1.4 spec and it
>> + * is prudent to disable all deep color modes. Return here before
>> + * committing bpc and edid_hdmi_dc_modes.
>> + */
>> +if (!(modes & DRM_EDID_HDMI_DC_36)) {
>> +DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
>> +  connector->name);
>> +return;
>> +}
>> +
>> +
>>  DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
>>connector->name, dc_bpc);
>>  info->bpc = dc_bpc;
>> +info->edid_hdmi_dc_modes = modes;

[PATCH] drm/probe-helpers: Drop locking from poll_enable

[Daniel Vetter]

It was only needed to protect the connector_list walking, see

commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
Author: Daniel Vetter 
Date:   Thu Jul 9 23:44:26 2015 +0200

drm/probe-helper: Grab mode_config.mutex in poll_init/enable

Unfortunately the commit message of that patch fails to mention that
the new locking check was for the connector_list.

But that requirement disappeared in

commit c36a3254f7857f1ad9badbe3578ccc92be541a8e
Author: Daniel Vetter 
Date:   Thu Dec 15 16:58:43 2016 +0100

drm: Convert all helpers to drm_connector_list_iter

and so we can drop this again.

This fixes a locking inversion on nouveau, where the rpm code needs to
re-enable. But in other places the rpm_get() calls are nested within
the big modeset locks.

While at it, also improve the kerneldoc for these two functions a
notch.

Cc: Dave Airlie 
Signed-off-by: Daniel Vetter 
---
 drivers/gpu/drm/drm_probe_helper.c   | 41 ++--
 drivers/gpu/drm/i915/intel_hotplug.c |  4 ++--
 include/drm/drm_crtc_helper.h|  1 -
 3 files changed, 13 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 060211ac74a1..7c70f2a52250 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -115,25 +115,24 @@ static int drm_helper_probe_add_cmdline_mode(struct 
drm_connector *connector)

 #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
 /**
- * drm_kms_helper_poll_enable_locked - re-enable output polling.
+ * drm_kms_helper_poll_enable - re-enable output polling.
  * @dev: drm_device
  *
- * This function re-enables the output polling work without
- * locking the mode_config mutex.
+ * This function re-enables the output polling work, after it has been
+ * temporarily disabled using drm_kms_helper_poll_disable(), for example over
+ * suspend/resume.
  *
- * This is like drm_kms_helper_poll_enable() however it is to be
- * called from a context where the mode_config mutex is locked
- * already.
+ * Drivers can call this helper from their device resume implementation. It is
+ * an error to call this when the output polling support has not yet been set
+ * up.
  */
-void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
+void drm_kms_helper_poll_enable(struct drm_device *dev)
 {
bool poll = false;
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
unsigned long delay = DRM_OUTPUT_POLL_PERIOD;

-   WARN_ON(!mutex_is_locked(>mode_config.mutex));
-
if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
return;

@@ -153,7 +152,7 @@ void drm_kms_helper_poll_enable_locked(struct drm_device 
*dev)
if (poll)
schedule_delayed_work(>mode_config.output_poll_work, 
delay);
 }
-EXPORT_SYMBOL(drm_kms_helper_poll_enable_locked);
+EXPORT_SYMBOL(drm_kms_helper_poll_enable);

 static enum drm_connector_status
 drm_connector_detect(struct drm_connector *connector, bool force)
@@ -280,7 +279,7 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,

/* Re-enable polling in case the global poll config changed. */
if (drm_kms_helper_poll != dev->mode_config.poll_running)
-   drm_kms_helper_poll_enable_locked(dev);
+   drm_kms_helper_poll_enable(dev);

dev->mode_config.poll_running = drm_kms_helper_poll;

@@ -475,7 +474,7 @@ static void output_poll_execute(struct work_struct *work)
  *
  * Drivers can call this helper from their device suspend implementation. It is
  * not an error to call this even when output polling isn't enabled or arlready
- * disabled.
+ * disabled. Polling is re-enabled by calling drm_kms_helper_poll_enable().
  */
 void drm_kms_helper_poll_disable(struct drm_device *dev)
 {
@@ -486,24 +485,6 @@ void drm_kms_helper_poll_disable(struct drm_device *dev)
 EXPORT_SYMBOL(drm_kms_helper_poll_disable);

 /**
- * drm_kms_helper_poll_enable - re-enable output polling.
- * @dev: drm_device
- *
- * This function re-enables the output polling work.
- *
- * Drivers can call this helper from their device resume implementation. It is
- * an error to call this when the output polling support has not yet been set
- * up.
- */
-void drm_kms_helper_poll_enable(struct drm_device *dev)
-{
-   mutex_lock(>mode_config.mutex);
-   drm_kms_helper_poll_enable_locked(dev);
-   mutex_unlock(>mode_config.mutex);
-}
-EXPORT_SYMBOL(drm_kms_helper_poll_enable);
-
-/**
  * drm_kms_helper_poll_init - initialize and enable output polling
  * @dev: drm_device
  *
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c 
b/drivers/gpu/drm/i915/intel_hotplug.c
index 2ddc9e5842ec..5122d4bfb70e 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -182,7 +182,7 @@ static void intel_hpd_irq_storm_disable(struct 
drm_i915_private *dev_priv)

/* Enable polling and queue hotplug re-enabling. */
if 

[Bug 99330] Severe flickering with Fiji on Wayland

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99330

--- Comment #5 from Vedran Miletić  ---
(In reply to Ernst Sjöstrand from comment #3)
> I happen to have 2 4K monitors and a Fury as well actually.
> 
> I actually couldn't run Wayland at all for some time, perhaps it was fixed
> by:
> mutter (3.22.2-3) unstable; urgency=medium
> 
>   [ Jeremy Bicha ]
>   * Add git_flush_all_swap_notifies_on_idle.patch:
> - Add patch from 3.22 branch that fixes freezes with multiple monitors
>   on Wayland
> landing in my Ubuntu repos.
> 

That's also in Fedora as far as I can tell:
https://koji.fedoraproject.org/koji/buildinfo?buildID=823521

(In reply to Ernst Sjöstrand from comment #4)
> Does it work with an older mesa/llvm version for you?

Mesa 13 and LLVM 3.8 in stock Fedora doesn't help.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/3b31b7f8/attachment.html>

[PATCH] drm: disable deep color when EDID violates spec

[Ville Syrjälä]

On Tue, Jan 10, 2017 at 12:33:35PM +0100, Ernst Sjöstrand wrote:
> Isn't 10bpc very common among monitors, and 12bpc very rare? Or maybe I'm
> confusing the transport layer with the presentation capabilities...?
> Here are 201 monitors that claim 10bpc:
> http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista

I suppose that refers to the panel? Not sure.

> 
> Regards
> //Ernst
> 
> 2017-01-10 11:52 GMT+01:00 Ville Syrjälä :
> 
> > On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
> > > As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
> > > greater than 24 bits per pixel. The spec explicitly states, "All Deep
> > > Color modes are optional though if an HDMI Source or Sink supports any
> > > Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
> > > Requirements).
> > >
> > > I came across a monitor (Acer X233H) that supplies an illegal EDID where
> > > DC_30bit is set and DC_36bit is not set. The end result is badly garbled
> > > output because the driver is sending 36BPP when the monitor can't handle
> > > it.
> > >
> > > Much of the intel hardware is incapable of operating at any
> > > bit-per-component setting outside of 8 or 12, and the spec seems to
> > > imply that if any deep color support is found, then it is a safe
> > > assumption to operate at 12.
> > >
> > > This patch ensures that the EDID is within the spec (specifically, that
> > > DC_36bit is set) before committing to going forward with any deep
> > > colors.  There was already a check for this EDID inconsistency, but it
> > > resulted only in a warning and did not fall-back to safer settings.
> > >
> > > CC: Ville Syrjälä 
> > > Signed-off-by: Nicholas Sielicki 
> > > ---
> > >  drivers/gpu/drm/drm_edid.c | 35 +++
> > >  1 file changed, 23 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 336be31ff3de..42ce3f54d2dc 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -3772,30 +3772,34 @@ static void drm_parse_hdmi_deep_color_info(struct
> > drm_connector *connector,
> > >  {
> > >   struct drm_display_info *info = >display_info;
> > >   unsigned int dc_bpc = 0;
> > > + u8 modes = 0;
> > >
> > >   /* HDMI supports at least 8 bpc */
> > >   info->bpc = 8;
> > >
> > > + /* Ensure all DC modes are unset if we return early */
> > > + info->edid_hdmi_dc_modes = 0;
> >
> > Clearing this in drm_add_display_info() should be sufficient since
> > this guy doesn't get called from anywhere else. So this part could
> > be droppped.
> >
> > Otherwise this feels like a decent way to handle the problem. We
> > could of course try to use the 10bpc (or whatever) deep color modes
> > the sink claims to support, but given that the people designing the
> > thing didn't bother reading the spec, it seems safer to just disable
> > deep color support entirely.
> >
> > Reviewed-by: Ville Syrjälä 
> >
> > > +
> > >   if (cea_db_payload_len(hdmi) < 6)
> > >   return;
> > >
> > >   if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
> > >   dc_bpc = 10;
> > > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
> > > + modes |= DRM_EDID_HDMI_DC_30;
> > >   DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
> > > connector->name);
> > >   }
> > >
> > >   if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
> > >   dc_bpc = 12;
> > > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
> > > + modes |= DRM_EDID_HDMI_DC_36;
> > >   DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
> > > connector->name);
> > >   }
> > >
> > >   if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
> > >   dc_bpc = 16;
> > > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
> > > + modes |= DRM_EDID_HDMI_DC_48;
> > >   DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
> > > connector->name);
> > >   }
> > > @@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_info(struct
> > drm_connector *connector,
> > >   return;
> > >   }
> > >
> > > + /*
> > > +  * All deep color modes are optional, but if a sink supports any
> > deep
> > > +  * color mode, it must support 36-bit mode. If this is found not
> > > +  * to be the case, sink is in violation of HDMI 1.3 / 1.4 spec and
> > it
> > > +  * is prudent to disable all deep color modes. Return here before
> > > +  * committing bpc and edid_hdmi_dc_modes.
> > > +  */
> > > + if (!(modes & DRM_EDID_HDMI_DC_36)) {
> > > + DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
> > > +   connector->name);
> > > + return;
> > > + }
> > > +
> > > +
> > >   DRM_DEBUG("%s: Assigning HDMI sink color depth as 

[Intel-gfx] [PATCH] drm/probe-helpers: Drop locking from poll_enable

[Chris Wilson]

On Tue, Jan 10, 2017 at 03:29:10PM +0100, Daniel Vetter wrote:
> It was only needed to protect the connector_list walking, see
> 
> commit 8c4ccc4ab6f64e859d4ff8d7c02c2ed2e956e07f
> Author: Daniel Vetter 
> Date:   Thu Jul 9 23:44:26 2015 +0200
> 
> drm/probe-helper: Grab mode_config.mutex in poll_init/enable
> 
> Unfortunately the commit message of that patch fails to mention that
> the new locking check was for the connector_list.
> 
> But that requirement disappeared in
> 
> commit c36a3254f7857f1ad9badbe3578ccc92be541a8e
> Author: Daniel Vetter 
> Date:   Thu Dec 15 16:58:43 2016 +0100
> 
> drm: Convert all helpers to drm_connector_list_iter
> 
> and so we can drop this again.
> 
> This fixes a locking inversion on nouveau, where the rpm code needs to
> re-enable. But in other places the rpm_get() calls are nested within
> the big modeset locks.
> 
> While at it, also improve the kerneldoc for these two functions a
> notch.
> 
> Cc: Dave Airlie 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_probe_helper.c   | 41 
> ++--
>  drivers/gpu/drm/i915/intel_hotplug.c |  4 ++--
>  include/drm/drm_crtc_helper.h|  1 -
>  3 files changed, 13 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> b/drivers/gpu/drm/drm_probe_helper.c
> index 060211ac74a1..7c70f2a52250 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -115,25 +115,24 @@ static int drm_helper_probe_add_cmdline_mode(struct 
> drm_connector *connector)
>  
>  #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
>  /**
> - * drm_kms_helper_poll_enable_locked - re-enable output polling.
> + * drm_kms_helper_poll_enable - re-enable output polling.
>   * @dev: drm_device
>   *
> - * This function re-enables the output polling work without
> - * locking the mode_config mutex.
> + * This function re-enables the output polling work, after it has been
> + * temporarily disabled using drm_kms_helper_poll_disable(), for example over
> + * suspend/resume.
>   *
> - * This is like drm_kms_helper_poll_enable() however it is to be
> - * called from a context where the mode_config mutex is locked
> - * already.
> + * Drivers can call this helper from their device resume implementation. It 
> is
> + * an error to call this when the output polling support has not yet been set
> + * up.
>   */
> -void drm_kms_helper_poll_enable_locked(struct drm_device *dev)
> +void drm_kms_helper_poll_enable(struct drm_device *dev)
>  {
>   bool poll = false;
>   struct drm_connector *connector;
>   struct drm_connector_list_iter conn_iter;
>   unsigned long delay = DRM_OUTPUT_POLL_PERIOD;
>  
> - WARN_ON(!mutex_is_locked(>mode_config.mutex));
> -
>   if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
>   return;

Hmm, output_poll_execute() itself is not checking this correctly,

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 84b75709af90..cb3febc6e719 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -405,7 +405,7 @@ static void output_poll_execute(struct work_struct *work)
changed = dev->mode_config.delayed_event;
dev->mode_config.delayed_event = false;

-   if (!drm_kms_helper_poll)
+   if (!dev->mode_config.poll_enabled || !drm_kms_helper_poll)
goto out;

if (!mutex_trylock(>mode_config.mutex)) {

The connector list is locked, but the other reads are all racy
(connector->polled, delayed_event). Those races seem intrinsic and handled
by e.g. intel_hotplug.c.

Since the locking here was only covering the connector list and that now
has its own lock,
Reviewed-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm: disable deep color when EDID violates spec

[Alex Deucher]

On Tue, Jan 10, 2017 at 3:02 PM, Ernst Sjöstrand  wrote:
> I kindof assume DP is the default connection these days and with Freesync
> you use
> DP or course, but this question was specifically for HDMI.
> I guess this patch doesn't affect deep color over DP?
>
> Anyway, only 17 of those monitors have FreeSync but almost all have DP, so
> perhaps they only support
> 10 bpc when connected with DP?

We see 10 bpc only over HDMI a lot apparently.  I guess a lot of
monitor vendors do the minimum necessary for deep color.

Alex

>
> Regards
> //Ernst
>
> 2017-01-10 20:54 GMT+01:00 Alex Deucher :
>>
>> On Tue, Jan 10, 2017 at 12:46 PM, Ville Syrjälä
>>  wrote:
>> > On Tue, Jan 10, 2017 at 12:27:45PM -0500, Alex Deucher wrote:
>> >> On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand 
>> >> wrote:
>> >> > Isn't 10bpc very common among monitors, and 12bpc very rare? Or maybe
>> >> > I'm
>> >> > confusing the transport layer with the presentation capabilities...?
>> >> > Here are 201 monitors that claim 10bpc:
>> >> > http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista
>> >> >
>> >>
>> >> FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
>> >> see quite a few 10 bpc monitors.
>> >
>> > I've seen plenty of monitors that do 10bpc over DP, but I've never seen
>> > anything that does 10bpc over HDMI. 12bpc over HDMI is pretty common
>> > in "proper" TVs in my experience.
>> >
>> >> I can talk to our display team to
>> >> see what other OSes do.
>> >
>> > Thanks. That should give us some idea if anyone ever tried 10bpc
>> > over HDMI on these things.
>>
>> We apparently see this pretty commonly, especially with freesync
>> monitors, and we support it.  It seems to be pretty prevalent because
>> you can support a higher refresh rate with a lower bpc.
>>
>> Alex
>>
>> >
>> >>
>> >> Alex
>> >>
>> >> > Regards
>> >> > //Ernst
>> >> >
>> >> > 2017-01-10 11:52 GMT+01:00 Ville Syrjälä
>> >> > :
>> >> >>
>> >> >> On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
>> >> >> > As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color
>> >> >> > depths
>> >> >> > greater than 24 bits per pixel. The spec explicitly states, "All
>> >> >> > Deep
>> >> >> > Color modes are optional though if an HDMI Source or Sink supports
>> >> >> > any
>> >> >> > Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
>> >> >> > Requirements).
>> >> >> >
>> >> >> > I came across a monitor (Acer X233H) that supplies an illegal EDID
>> >> >> > where
>> >> >> > DC_30bit is set and DC_36bit is not set. The end result is badly
>> >> >> > garbled
>> >> >> > output because the driver is sending 36BPP when the monitor can't
>> >> >> > handle
>> >> >> > it.
>> >> >> >
>> >> >> > Much of the intel hardware is incapable of operating at any
>> >> >> > bit-per-component setting outside of 8 or 12, and the spec seems
>> >> >> > to
>> >> >> > imply that if any deep color support is found, then it is a safe
>> >> >> > assumption to operate at 12.
>> >> >> >
>> >> >> > This patch ensures that the EDID is within the spec (specifically,
>> >> >> > that
>> >> >> > DC_36bit is set) before committing to going forward with any deep
>> >> >> > colors.  There was already a check for this EDID inconsistency,
>> >> >> > but it
>> >> >> > resulted only in a warning and did not fall-back to safer
>> >> >> > settings.
>> >> >> >
>> >> >> > CC: Ville Syrjälä 
>> >> >> > Signed-off-by: Nicholas Sielicki 
>> >> >> > ---
>> >> >> >  drivers/gpu/drm/drm_edid.c | 35
>> >> >> > +++
>> >> >> >  1 file changed, 23 insertions(+), 12 deletions(-)
>> >> >> >
>> >> >> > diff --git a/drivers/gpu/drm/drm_edid.c
>> >> >> > b/drivers/gpu/drm/drm_edid.c
>> >> >> > index 336be31ff3de..42ce3f54d2dc 100644
>> >> >> > --- a/drivers/gpu/drm/drm_edid.c
>> >> >> > +++ b/drivers/gpu/drm/drm_edid.c
>> >> >> > @@ -3772,30 +3772,34 @@ static void
>> >> >> > drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>> >> >> >  {
>> >> >> >   struct drm_display_info *info = >display_info;
>> >> >> >   unsigned int dc_bpc = 0;
>> >> >> > + u8 modes = 0;
>> >> >> >
>> >> >> >   /* HDMI supports at least 8 bpc */
>> >> >> >   info->bpc = 8;
>> >> >> >
>> >> >> > + /* Ensure all DC modes are unset if we return early */
>> >> >> > + info->edid_hdmi_dc_modes = 0;
>> >> >>
>> >> >> Clearing this in drm_add_display_info() should be sufficient since
>> >> >> this guy doesn't get called from anywhere else. So this part could
>> >> >> be droppped.
>> >> >>
>> >> >> Otherwise this feels like a decent way to handle the problem. We
>> >> >> could of course try to use the 10bpc (or whatever) deep color modes
>> >> >> the sink claims to support, but given that the people designing the
>> >> >> thing didn't bother reading the spec, it seems safer to just disable
>> >> >> deep color support entirely.
>> >> >>
>> >> >> Reviewed-by: Ville Syrjälä 
>> >> >>
>> >> >> > +
>> >> >> >   if 

[PATCH] drm: disable deep color when EDID violates spec

[Alex Deucher]

On Tue, Jan 10, 2017 at 12:46 PM, Ville Syrjälä
 wrote:
> On Tue, Jan 10, 2017 at 12:27:45PM -0500, Alex Deucher wrote:
>> On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand  
>> wrote:
>> > Isn't 10bpc very common among monitors, and 12bpc very rare? Or maybe I'm
>> > confusing the transport layer with the presentation capabilities...?
>> > Here are 201 monitors that claim 10bpc:
>> > http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista
>> >
>>
>> FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
>> see quite a few 10 bpc monitors.
>
> I've seen plenty of monitors that do 10bpc over DP, but I've never seen
> anything that does 10bpc over HDMI. 12bpc over HDMI is pretty common
> in "proper" TVs in my experience.
>
>> I can talk to our display team to
>> see what other OSes do.
>
> Thanks. That should give us some idea if anyone ever tried 10bpc
> over HDMI on these things.

We apparently see this pretty commonly, especially with freesync
monitors, and we support it.  It seems to be pretty prevalent because
you can support a higher refresh rate with a lower bpc.

Alex

>
>>
>> Alex
>>
>> > Regards
>> > //Ernst
>> >
>> > 2017-01-10 11:52 GMT+01:00 Ville Syrjälä > > linux.intel.com>:
>> >>
>> >> On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
>> >> > As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
>> >> > greater than 24 bits per pixel. The spec explicitly states, "All Deep
>> >> > Color modes are optional though if an HDMI Source or Sink supports any
>> >> > Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
>> >> > Requirements).
>> >> >
>> >> > I came across a monitor (Acer X233H) that supplies an illegal EDID where
>> >> > DC_30bit is set and DC_36bit is not set. The end result is badly garbled
>> >> > output because the driver is sending 36BPP when the monitor can't handle
>> >> > it.
>> >> >
>> >> > Much of the intel hardware is incapable of operating at any
>> >> > bit-per-component setting outside of 8 or 12, and the spec seems to
>> >> > imply that if any deep color support is found, then it is a safe
>> >> > assumption to operate at 12.
>> >> >
>> >> > This patch ensures that the EDID is within the spec (specifically, that
>> >> > DC_36bit is set) before committing to going forward with any deep
>> >> > colors.  There was already a check for this EDID inconsistency, but it
>> >> > resulted only in a warning and did not fall-back to safer settings.
>> >> >
>> >> > CC: Ville Syrjälä 
>> >> > Signed-off-by: Nicholas Sielicki 
>> >> > ---
>> >> >  drivers/gpu/drm/drm_edid.c | 35 +++
>> >> >  1 file changed, 23 insertions(+), 12 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> >> > index 336be31ff3de..42ce3f54d2dc 100644
>> >> > --- a/drivers/gpu/drm/drm_edid.c
>> >> > +++ b/drivers/gpu/drm/drm_edid.c
>> >> > @@ -3772,30 +3772,34 @@ static void
>> >> > drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>> >> >  {
>> >> >   struct drm_display_info *info = >display_info;
>> >> >   unsigned int dc_bpc = 0;
>> >> > + u8 modes = 0;
>> >> >
>> >> >   /* HDMI supports at least 8 bpc */
>> >> >   info->bpc = 8;
>> >> >
>> >> > + /* Ensure all DC modes are unset if we return early */
>> >> > + info->edid_hdmi_dc_modes = 0;
>> >>
>> >> Clearing this in drm_add_display_info() should be sufficient since
>> >> this guy doesn't get called from anywhere else. So this part could
>> >> be droppped.
>> >>
>> >> Otherwise this feels like a decent way to handle the problem. We
>> >> could of course try to use the 10bpc (or whatever) deep color modes
>> >> the sink claims to support, but given that the people designing the
>> >> thing didn't bother reading the spec, it seems safer to just disable
>> >> deep color support entirely.
>> >>
>> >> Reviewed-by: Ville Syrjälä 
>> >>
>> >> > +
>> >> >   if (cea_db_payload_len(hdmi) < 6)
>> >> >   return;
>> >> >
>> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
>> >> >   dc_bpc = 10;
>> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
>> >> > + modes |= DRM_EDID_HDMI_DC_30;
>> >> >   DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
>> >> > connector->name);
>> >> >   }
>> >> >
>> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
>> >> >   dc_bpc = 12;
>> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
>> >> > + modes |= DRM_EDID_HDMI_DC_36;
>> >> >   DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
>  >> > connector->name);
>> >> >   }
>> >> >
>> >> >   if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
>> >> >   dc_bpc = 16;
>> >> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
>> >> > + modes |= DRM_EDID_HDMI_DC_48;
>> >> >   DRM_DEBUG("%s: 

DRM range manger test failures

[Daniel Vetter]

On Tue, Jan 10, 2017 at 12:45:04PM +, Chris Wilson wrote:
> On Tue, Jan 10, 2017 at 01:41:54PM +0100, Geert Uytterhoeven wrote:
> > Hi Chris, Laurent,
> > 
> > On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
> > manager selftests fail with:
> > 
> > drm_mm: Testing DRM range manger (struct drm_mm), with
> > random_seed=0x83a9b910 max_iterations=8192 max_prime=128
> > drm_mm: igt_sanitycheck - ok!
> > drm_mm: node has wrong color, found 0, expected 1
> > drm_mm: default insert failed, size 1 step 1
> > 
> > I have no idea what this means.
> 
> The existing code has bugs. We got as far as applying the testcases to
> demonstrate the bugs, the actual fix is still queued.

Hm, I thought I merged them all. What's missing?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v1 7/7] ARM: configs: Add STM32 LTDC support in STM32 defconfig

[Yannick Fertre]

Signed-off-by: Yannick Fertre 
---
 arch/arm/configs/stm32_defconfig | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig
index 29068f5..e3974d9 100644
--- a/arch/arm/configs/stm32_defconfig
+++ b/arch/arm/configs/stm32_defconfig
@@ -71,3 +71,8 @@ CONFIG_MAGIC_SYSRQ=y
 # CONFIG_FTRACE is not set
 CONFIG_CRC_ITU_T=y
 CONFIG_CRC7=y
+CONFIG_DRM=y
+CONFIG_DRM_ST=y
+CONFIG_DRM_PANEL=y
+CONFIG_DRM_PANEL_SIMPLE=y
+CONFIG_BACKLIGHT_LCD_SUPPORT=y
-- 
1.9.1

[PATCH v1 6/7] ARM: dts: stm32429i-eval: Enable ltdc & simple panel on Eval board

[Yannick Fertre]

Enable ltdc & enable am-480272h3tmqw-t01h panel.

Signed-off-by: Yannick Fertre 
---
 arch/arm/boot/dts/stm32429i-eval.dts | 58 
 1 file changed, 58 insertions(+)

diff --git a/arch/arm/boot/dts/stm32429i-eval.dts 
b/arch/arm/boot/dts/stm32429i-eval.dts
index 2de6487..f987ca5 100644
--- a/arch/arm/boot/dts/stm32429i-eval.dts
+++ b/arch/arm/boot/dts/stm32429i-eval.dts
@@ -88,6 +88,52 @@
clocks = < 0 30>;
clock-names = "main_clk";
};
+
+   panel_rgb: panel-rgb {
+   compatible = "ampire,am-480272h3tmqw-t01h";
+   status = "okay";
+   port {
+   panel_in_rgb: endpoint {
+   remote-endpoint = <_out_rgb>;
+   };
+   };
+   };
+};
+
+ {
+   pinctrl_ltdc: ltdc at 0 {
+   pins {
+   pinmux = ,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+,
+;
+   slew-rate = <2>;
+   };
+   };
 };

 _hse {
@@ -123,3 +169,15 @@
pinctrl-names = "default";
status = "okay";
 };
+
+_host{
+   status = "okay";
+   pinctrl-0 = <_ltdc>;
+   pinctrl-names = "default";
+
+   port {
+   ltdc_out_rgb: endpoint {
+   remote-endpoint = <_in_rgb>;
+   };
+   };
+};
-- 
1.9.1

[PATCH v1 5/7] ARM: dts: stm32f429: Add ltdc support

[Yannick Fertre]

Add LTDC (Lcd-tft Display Controller) support.

Signed-off-by: Yannick Fertre 
---
 arch/arm/boot/dts/stm32f429.dtsi | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/stm32f429.dtsi b/arch/arm/boot/dts/stm32f429.dtsi
index 336ee4f..fc43415 100644
--- a/arch/arm/boot/dts/stm32f429.dtsi
+++ b/arch/arm/boot/dts/stm32f429.dtsi
@@ -185,7 +185,7 @@
interrupts = <1>, <2>, <3>, <6>, <7>, <8>, <9>, <10>, 
<23>, <40>, <41>, <42>, <62>, <76>;
};

-   pin-controller {
+   pinctrl: pin-controller {
#address-cells = <1>;
#size-cells = <1>;
compatible = "st,stm32f429-pinctrl";
@@ -404,6 +404,29 @@
interrupts = <80>;
clocks = < 0 38>;
};
+
+   st-display-subsystem {
+   compatible = "st,display-subsystem";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+   dma-ranges;
+
+   ltdc_host: stm32-ltdc at 40016800 {
+   compatible = "st,ltdc";
+   reg = <0x40016800 0x200>;
+   interrupts = <88>, <89>;
+   resets = < 314>;
+   clocks = < 1 8>;
+   clock-names = "clk-lcd";
+   status = "disabled";
+
+   port {
+   ltdc_out_rgb: endpoint {
+   };
+   };
+   };
+   };
};
 };

-- 
1.9.1

[PATCH v1 4/7] drm/panel: simple: Add support for Ampire AM-480272H3TMQW-T01H

[Yannick Fertre]

Add simple-panel support for the Ampire AM-480272H3TMQW-T01H,
which is a 4.3" WQVGA panel.

Signed-off-by: Yannick Fertre 
---
 drivers/gpu/drm/panel/panel-simple.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/panel/panel-simple.c 
b/drivers/gpu/drm/panel/panel-simple.c
index 06aaf79..ee5d2ff 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -386,6 +386,32 @@ static void panel_simple_shutdown(struct device *dev)
panel_simple_disable(>base);
 }

+static const struct drm_display_mode ampire_am_480272h3tmqw_t01h_mode = {
+   .clock = 9000,
+   .hdisplay = 480,
+   .hsync_start = 480 + 2,
+   .hsync_end = 480 + 2 + 41,
+   .htotal = 480 + 2 + 41 + 2,
+   .vdisplay = 272,
+   .vsync_start = 272 + 2,
+   .vsync_end = 272 + 2 + 10,
+   .vtotal = 272 + 2 + 10 + 2,
+   .vrefresh = 60,
+   .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
+};
+
+static const struct panel_desc ampire_am_480272h3tmqw_t01h = {
+   .modes = _am_480272h3tmqw_t01h_mode,
+   .num_modes = 1,
+   .bpc = 8,
+
+   .size = {
+   .width = 105,
+   .height = 67,
+   },
+   .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+};
+
 static const struct drm_display_mode ampire_am800480r3tmqwa1h_mode = {
.clock = 3,
.hdisplay = 800,
@@ -1715,6 +1741,9 @@ static void panel_simple_shutdown(struct device *dev)

 static const struct of_device_id platform_of_match[] = {
{
+   .compatible = "ampire,am-480272h3tmqw-t01h",
+   .data = _am_480272h3tmqw_t01h,
+   }, {
.compatible = "ampire,am800480r3tmqwa1h",
.data = _am800480r3tmqwa1h,
}, {
-- 
1.9.1

[PATCH v1 3/7] dt-bindings: Add Ampire AM-480272H3TMQW-T01H panel

[Yannick Fertre]

Signed-off-by: Yannick Fertre 
---
 .../bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt | 7 +++
 1 file changed, 7 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt

diff --git 
a/Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt
 
b/Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt
new file mode 100644
index 000..f59e3c4
--- /dev/null
+++ 
b/Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt
@@ -0,0 +1,7 @@
+Ampire AM-480272H3TMQW-T01H 4.3" WQVGA TFT LCD panel
+
+Required properties:
+- compatible: should be "ampire,am-480272h3tmqw-t01h"
+
+This binding is compatible with the simple-panel binding, which is specified
+in simple-panel.txt in this directory.
-- 
1.9.1

[PATCH v1 2/7] drm/st: Add STM32 LTDC driver

[Yannick Fertre]

This patch adds support for the STM32 LCD-TFT display controller.

Signed-off-by: Yannick Fertre 
---
 drivers/gpu/drm/Kconfig |2 +
 drivers/gpu/drm/Makefile|1 +
 drivers/gpu/drm/st/Kconfig  |   14 +
 drivers/gpu/drm/st/Makefile |7 +
 drivers/gpu/drm/st/drv.c|  279 +
 drivers/gpu/drm/st/drv.h|   25 +
 drivers/gpu/drm/st/ltdc.c   | 1438 +++
 drivers/gpu/drm/st/ltdc.h   |   20 +
 8 files changed, 1786 insertions(+)
 create mode 100644 drivers/gpu/drm/st/Kconfig
 create mode 100644 drivers/gpu/drm/st/Makefile
 create mode 100644 drivers/gpu/drm/st/drv.c
 create mode 100644 drivers/gpu/drm/st/drv.h
 create mode 100644 drivers/gpu/drm/st/ltdc.c
 create mode 100644 drivers/gpu/drm/st/ltdc.h

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 6f3f9e6..d8e6f92 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -237,6 +237,8 @@ source "drivers/gpu/drm/fsl-dcu/Kconfig"

 source "drivers/gpu/drm/tegra/Kconfig"

+source "drivers/gpu/drm/st/Kconfig"
+
 source "drivers/gpu/drm/panel/Kconfig"

 source "drivers/gpu/drm/bridge/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 92de399..7434c09 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_DRM_BOCHS) += bochs/
 obj-$(CONFIG_DRM_VIRTIO_GPU) += virtio/
 obj-$(CONFIG_DRM_MSM) += msm/
 obj-$(CONFIG_DRM_TEGRA) += tegra/
+obj-$(CONFIG_DRM_ST) += st/
 obj-$(CONFIG_DRM_STI) += sti/
 obj-$(CONFIG_DRM_IMX) += imx/
 obj-$(CONFIG_DRM_MEDIATEK) += mediatek/
diff --git a/drivers/gpu/drm/st/Kconfig b/drivers/gpu/drm/st/Kconfig
new file mode 100644
index 000..fa0ac0c
--- /dev/null
+++ b/drivers/gpu/drm/st/Kconfig
@@ -0,0 +1,14 @@
+config DRM_ST
+   tristate "DRM Support for STMicroelectronics SoC Series"
+   depends on DRM && (ARCH_STM32 || ARCH_MULTIPLATFORM)
+   select DRM_KMS_HELPER
+   select DRM_GEM_CMA_HELPER
+   select DRM_KMS_CMA_HELPER
+   select DRM_PANEL
+   select VIDEOMODE_HELPERS
+   select FB_PROVIDE_GET_FB_UNMAPPED_AREA
+   help
+ Choose this option if you have an ST STMicroelectronics SoC.
+
+ To compile this driver as a module, choose M here: the module
+ will be called st-drm.
diff --git a/drivers/gpu/drm/st/Makefile b/drivers/gpu/drm/st/Makefile
new file mode 100644
index 000..b2a9025
--- /dev/null
+++ b/drivers/gpu/drm/st/Makefile
@@ -0,0 +1,7 @@
+ccflags-y := -Iinclude/drm
+
+st-drm-y := \
+   drv.o \
+   ltdc.o
+
+obj-$(CONFIG_DRM_ST) += st-drm.o
diff --git a/drivers/gpu/drm/st/drv.c b/drivers/gpu/drm/st/drv.c
new file mode 100644
index 000..a275019
--- /dev/null
+++ b/drivers/gpu/drm/st/drv.c
@@ -0,0 +1,279 @@
+/*
+ * Copyright (C) STMicroelectronics SA 2017
+ *
+ * Authors: Philippe Cornu 
+ *  Yannick Fertre 
+ *  Fabien Dessenne 
+ *  Mickael Reulier 
+ *
+ * License terms:  GNU General Public License (GPL), version 2
+ */
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "drv.h"
+#include "ltdc.h"
+
+#define DRIVER_NAME"st"
+#define DRIVER_DESC"STMicroelectronics SoC DRM"
+#define DRIVER_DATE"20170110"
+#define DRIVER_MAJOR   1
+#define DRIVER_MINOR   0
+
+#define ST_MAX_FB_WIDTH2048
+#define ST_MAX_FB_HEIGHT   2048 /* same as width to handle orientation */
+
+static void st_output_poll_changed(struct drm_device *ddev)
+{
+   struct st_private *priv = ddev->dev_private;
+
+   drm_fbdev_cma_hotplug_event(priv->fbdev);
+}
+
+static const struct drm_mode_config_funcs st_mode_config_funcs = {
+   .fb_create = drm_fb_cma_create,
+   .output_poll_changed = st_output_poll_changed,
+   .atomic_check = drm_atomic_helper_check,
+   .atomic_commit = drm_atomic_helper_commit,
+};
+
+static void st_mode_config_init(struct drm_device *ddev)
+{
+   ddev->mode_config.min_width = 0;
+   ddev->mode_config.min_height = 0;
+
+   /*
+* set max width and height as default value.
+* this value would be used to check framebuffer size limitation
+* at drm_mode_addfb().
+*/
+   ddev->mode_config.max_width = ST_MAX_FB_WIDTH;
+   ddev->mode_config.max_height = ST_MAX_FB_HEIGHT;
+   ddev->mode_config.funcs = _mode_config_funcs;
+}
+
+static const struct file_operations st_driver_fops = {
+   .owner = THIS_MODULE,
+   .open = drm_open,
+   .get_unmapped_area = drm_gem_cma_get_unmapped_area,
+   .mmap = drm_gem_cma_mmap,
+   .poll = drm_poll,
+   .read = drm_read,
+   .unlocked_ioctl = drm_ioctl,
+#ifdef CONFIG_COMPAT
+   .compat_ioctl = drm_compat_ioctl,
+#endif
+   .release = drm_release,
+};
+
+static struct drm_driver st_driver = {
+   .driver_features = DRIVER_

[PATCH v1 1/7] dt-bindings: display: add STM32 LTDC driver

[Yannick Fertre]

Signed-off-by: Yannick Fertre 
---
 .../devicetree/bindings/display/st,ltdc.txt| 57 ++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/st,ltdc.txt

diff --git a/Documentation/devicetree/bindings/display/st,ltdc.txt 
b/Documentation/devicetree/bindings/display/st,ltdc.txt
new file mode 100644
index 000..20e89da
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/st,ltdc.txt
@@ -0,0 +1,57 @@
+* STMicroelectronics STM32 lcd-tft display controller
+
+- st-display-subsystem: Master device for DRM sub-components
+  This device must be the parent of all the sub-components and is responsible
+  of bind them.
+  Required properties:
+  - compatible: "st,display-subsystem"
+  - ranges: to allow probing of subdevices
+
+- ltdc_host: lcd-tft display controller host
+  must be a sub-node of st-display-subsystem
+  Required properties:
+  - compatible: "st,ltdc"
+  - reg: Physical base address of the IP registers and length of memory mapped 
region.
+  - clocks: from common clock binding: handle hardware IP needed clocks, the
+number of clocks may depend of the SoC type.
+See ../clocks/clock-bindings.txt for details.
+  - clock-names: names of the clocks listed in clocks property in the same
+order.
+  - resets: resets to be used by the device
+See ../reset/reset.txt for details.
+  - reset-names: names of the resets listed in resets property in the same
+order.
+  Required nodes:
+- Video port for RGB output.
+
+Example:
+
+/ {
+   ...
+   soc {
+   ...
+   st-display-subsystem {
+   compatible = "st,display-subsystem";
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+   dma-ranges;
+
+   ltdc_host: stm32-ltdc at 40016800 {
+   compatible = "st,ltdc";
+   reg = <0x40016800 0x200>;
+   interrupts = <88>, <89>;
+   resets = < 314>;
+   clocks = < 1 8>;
+   clock-names = "clk-lcd";
+   status = "disabled";
+
+   port {
+   ltdc_out_rgb: endpoint {
+   };
+   };
+   };
+   };
+   ...
+   };
+};
-- 
1.9.1

[PATCH v1 0/7] DRM: add LTDC support for STM32F4

[Yannick Fertre]

The purpose of this set of patches is to add a new driver for stm32f429.
This driver was developed and tested on evaluation board stm32429i.

Stm32f4 is a MCU platform which don't have MMU so the last patches developed
by Benjamin Gaignard regarding "DRM: allow to use mmuless devices"
are necessary.

The board stm429i embeds a Ampire AM-480272H3TMQW-T01H screen.
A new simple panel am-480272h3tmqw-t01h have been added to support it.

Yannick Fertre (7):
  dt-bindings: display: add STM32 LTDC driver
  drm/st: Add STM32 LTDC driver
  dt-bindings: Add Ampire AM-480272H3TMQW-T01H panel
  drm/panel: simple: Add support for Ampire AM-480272H3TMQW-T01H
  ARM: dts: stm32f429: Add ltdc support
  ARM: dts: stm32429i-eval: Enable ltdc & simple panel on Eval board
  ARM: configs: Add STM32 LTDC support in STM32 defconfig

 .../display/panel/ampire,am-480272h3tmqw-t01h.txt  |7 +
 .../devicetree/bindings/display/st,ltdc.txt|   57 +
 arch/arm/boot/dts/stm32429i-eval.dts   |   58 +
 arch/arm/boot/dts/stm32f429.dtsi   |   25 +-
 arch/arm/configs/stm32_defconfig   |5 +
 drivers/gpu/drm/Kconfig|2 +
 drivers/gpu/drm/Makefile   |1 +
 drivers/gpu/drm/panel/panel-simple.c   |   29 +
 drivers/gpu/drm/st/Kconfig |   14 +
 drivers/gpu/drm/st/Makefile|7 +
 drivers/gpu/drm/st/drv.c   |  279 
 drivers/gpu/drm/st/drv.h   |   25 +
 drivers/gpu/drm/st/ltdc.c  | 1438 
 drivers/gpu/drm/st/ltdc.h  |   20 +
 14 files changed, 1966 insertions(+), 1 deletion(-)
 create mode 100644 
Documentation/devicetree/bindings/display/panel/ampire,am-480272h3tmqw-t01h.txt
 create mode 100644 Documentation/devicetree/bindings/display/st,ltdc.txt
 create mode 100644 drivers/gpu/drm/st/Kconfig
 create mode 100644 drivers/gpu/drm/st/Makefile
 create mode 100644 drivers/gpu/drm/st/drv.c
 create mode 100644 drivers/gpu/drm/st/drv.h
 create mode 100644 drivers/gpu/drm/st/ltdc.c
 create mode 100644 drivers/gpu/drm/st/ltdc.h

-- 
1.9.1

[PATCH v2.1 1/7] drm/atomic: Add new iterators over all state, v3.

[Maarten Lankhorst]

Add for_each_(old)(new)_(plane,connector,crtc)_in_state iterators to
replace the old for_each_xxx_in_state ones. This is useful for >1 flip
depth and getting rid of all xxx->state dereferences.

This requires extra fixups done when committing a state after
duplicating, which in general isn't valid but is used by suspend/resume.
To handle these, introduce drm_atomic_helper_commit_duplicated_state
which performs those fixups before checking & committing the state.

Changes since v1:
- Remove nonblock parameter for commit_duplicated_state.
Changes since v2:
- Use commit_duplicated_state for i915 load detection.
- Add WARN_ON(old_state != obj->state) before swapping.

Signed-off-by: Maarten Lankhorst 
---
Forgot that there was a v2. It still broke on igt load detect tests, this 
should be the fix.

I also added a big WARN_ON before swapping to ensure old state is correct. At 
this point we
can no longer fix it because atomic_check is called with the wrong items, but 
it will help
limit the damage.

 drivers/gpu/drm/drm_atomic.c |  6 +++
 drivers/gpu/drm/drm_atomic_helper.c  | 65 +
 drivers/gpu/drm/i915/intel_display.c | 13 +++---
 include/drm/drm_atomic.h | 81 ++--
 include/drm/drm_atomic_helper.h  |  2 +
 5 files changed, 149 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 6414bcf7f41b..1c1cbf436717 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -275,6 +275,8 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
return ERR_PTR(-ENOMEM);

state->crtcs[index].state = crtc_state;
+   state->crtcs[index].old_state = crtc->state;
+   state->crtcs[index].new_state = crtc_state;
state->crtcs[index].ptr = crtc;
crtc_state->state = state;

@@ -691,6 +693,8 @@ drm_atomic_get_plane_state(struct drm_atomic_state *state,

state->planes[index].state = plane_state;
state->planes[index].ptr = plane;
+   state->planes[index].old_state = plane->state;
+   state->planes[index].new_state = plane_state;
plane_state->state = state;

DRM_DEBUG_ATOMIC("Added [PLANE:%d:%s] %p state to %p\n",
@@ -1031,6 +1035,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state 
*state,

drm_connector_reference(connector);
state->connectors[index].state = connector_state;
+   state->connectors[index].old_state = connector->state;
+   state->connectors[index].new_state = connector_state;
state->connectors[index].ptr = connector;
connector_state->state = state;

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index 7b71ac48b8a4..b652ef1a25b1 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1971,11 +1971,11 @@ void drm_atomic_helper_swap_state(struct 
drm_atomic_state *state,
int i;
long ret;
struct drm_connector *connector;
-   struct drm_connector_state *conn_state;
+   struct drm_connector_state *conn_state, *old_conn_state;
struct drm_crtc *crtc;
-   struct drm_crtc_state *crtc_state;
+   struct drm_crtc_state *crtc_state, *old_crtc_state;
struct drm_plane *plane;
-   struct drm_plane_state *plane_state;
+   struct drm_plane_state *plane_state, *old_plane_state;
struct drm_crtc_commit *commit;

if (stall) {
@@ -1999,13 +1999,17 @@ void drm_atomic_helper_swap_state(struct 
drm_atomic_state *state,
}
}

-   for_each_connector_in_state(state, connector, conn_state, i) {
+   for_each_oldnew_connector_in_state(state, connector, old_conn_state, 
conn_state, i) {
+   WARN_ON(connector->state != old_conn_state);
+
connector->state->state = state;
swap(state->connectors[i].state, connector->state);
connector->state->state = NULL;
}

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, crtc_state, 
i) {
+   WARN_ON(crtc->state != old_crtc_state);
+
crtc->state->state = state;
swap(state->crtcs[i].state, crtc->state);
crtc->state->state = NULL;
@@ -2020,7 +2024,9 @@ void drm_atomic_helper_swap_state(struct drm_atomic_state 
*state,
}
}

-   for_each_plane_in_state(state, plane, plane_state, i) {
+   for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
plane_state, i) {
+   WARN_ON(plane->state != old_plane_state);
+
plane->state->state = state;
swap(state->planes[i].state, plane->state);
plane->state->state = NULL;
@@ -2471,7 +2477,7 @@ EXPORT_SYMBOL(drm_atomic_helper_disable_all);
  *
  * See also:
  * drm_atomic_helper_duplicate_state(), 

[RESEND PATCH v14 2/2] drm/i915: Put "cooked" vlank counters in frame CRC lines

[Tomeu Vizoso]

Use drm_accurate_vblank_count so we have the full 32 bit to represent
the frame counter and userspace has a simpler way of knowing when the
counter wraps around.

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Emil Velikov 
Reviewed-by: Robert Foss 
---

 drivers/gpu/drm/i915/i915_irq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b9beb5955dae..75fb1f66cc0c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1557,7 +1557,6 @@ static void display_pipe_crc_irq_handler(struct 
drm_i915_private *dev_priv,
struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
int head, tail;
-   u32 frame;

spin_lock(_crc->lock);
if (pipe_crc->source) {
@@ -1612,8 +1611,9 @@ static void display_pipe_crc_irq_handler(struct 
drm_i915_private *dev_priv,
crcs[2] = crc2;
crcs[3] = crc3;
crcs[4] = crc4;
-   frame = driver->get_vblank_counter(_priv->drm, pipe);
-   drm_crtc_add_crc_entry(>base, true, frame, crcs);
+   drm_crtc_add_crc_entry(>base, true,
+  drm_accurate_vblank_count(>base),
+  crcs);
}
 }
 #else
-- 
2.9.3

[RESEND PATCH v14 1/2] drm/i915: Use new CRC debugfs API

[Tomeu Vizoso]

The core provides now an ABI to userspace for generation of frame CRCs,
so implement the ->set_crc_source() callback and reuse as much code as
possible with the previous ABI implementation.

When handling the pageflip interrupt, we skip 1 or 2 frames depending on
the HW because they contain wrong values. For the legacy ABI for
generating frame CRCs, this was done in userspace but now that we have a
generic ABI it's better if it's not exposed by the kernel.

v2:
- Leave the legacy implementation in place as the ABI implementation
  in the core is incompatible with it.
v3:
- Use the "cooked" vblank counter so we have a whole 32 bits.
- Make sure we don't mess with the state of the legacy CRC capture
  ABI implementation.
v4:
- Keep use of get_vblank_counter as in the legacy code, will be
  changed in a followup commit.

v5:
- Skip first frame or two as it's known that they contain wrong
  data.
- A few fixes suggested by Emil Velikov.

v6:
- Rework programming of the HW registers to preserve previous
  behavior.

v7:
- Address whitespace issue.
- Added a comment on why in the implementation of the new ABI we
  skip the 1st or 2nd frames.

v9:
- Add stub for intel_crtc_set_crc_source.

v12:
- Rebased.
- Remove stub for intel_crtc_set_crc_source and instead set the
  callback to NULL (Jani Nikula).

v15:
- Rebased.

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Emil Velikov 
Reviewed-by: Robert Foss 

irq

---

 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 77 ++--
 drivers/gpu/drm/i915/intel_display.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h  |  6 +++
 drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++-
 5 files changed, 142 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ffeebf869e36..7a0eab675031 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1810,6 +1810,7 @@ struct intel_pipe_crc {
enum intel_pipe_crc_source source;
int head, tail;
wait_queue_head_t wq;
+   int skipped;
 };

 struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a0e70f5b3aad..b9beb5955dae 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1553,41 +1553,68 @@ static void display_pipe_crc_irq_handler(struct 
drm_i915_private *dev_priv,
 {
struct intel_pipe_crc *pipe_crc = _priv->pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
+   struct intel_crtc *crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
+   struct drm_driver *driver = dev_priv->drm.driver;
+   uint32_t crcs[5];
int head, tail;
+   u32 frame;

spin_lock(_crc->lock);
+   if (pipe_crc->source) {
+   if (!pipe_crc->entries) {
+   spin_unlock(_crc->lock);
+   DRM_DEBUG_KMS("spurious interrupt\n");
+   return;
+   }

-   if (!pipe_crc->entries) {
-   spin_unlock(_crc->lock);
-   DRM_DEBUG_KMS("spurious interrupt\n");
-   return;
-   }
-
-   head = pipe_crc->head;
-   tail = pipe_crc->tail;
+   head = pipe_crc->head;
+   tail = pipe_crc->tail;

-   if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
-   spin_unlock(_crc->lock);
-   DRM_ERROR("CRC buffer overflowing\n");
-   return;
-   }
+   if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
+   spin_unlock(_crc->lock);
+   DRM_ERROR("CRC buffer overflowing\n");
+   return;
+   }

-   entry = _crc->entries[head];
+   entry = _crc->entries[head];

-   entry->frame = dev_priv->drm.driver->get_vblank_counter(_priv->drm,
-pipe);
-   entry->crc[0] = crc0;
-   entry->crc[1] = crc1;
-   entry->crc[2] = crc2;
-   entry->crc[3] = crc3;
-   entry->crc[4] = crc4;
+   entry->frame = driver->get_vblank_counter(_priv->drm, pipe);
+   entry->crc[0] = crc0;
+   entry->crc[1] = crc1;
+   entry->crc[2] = crc2;
+   entry->crc[3] = crc3;
+   entry->crc[4] = crc4;

-   head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
-   pipe_crc->head = head;
+   head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+   pipe_crc->head = head;

-   spin_unlock(_crc->lock);
+   spin_unlock(_crc->lock);

-   wake_up_interruptible(_crc->wq);
+   wake_up_interruptible(_crc->wq);
+   } else {
+   /*
+* For some not yet identified reason, the first CRC 

[RESEND PATCH v14 0/2] New debugfs API for capturing CRC of frames

[Tomeu Vizoso]

Hi,

here are the last two patches that remain to be merged in this series,
rebased on today's drm-tip.

Thanks,

Tomeu


Tomeu Vizoso (2):
  drm/i915: Use new CRC debugfs API
  drm/i915: Put "cooked" vlank counters in frame CRC lines

 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 77 ++--
 drivers/gpu/drm/i915/intel_display.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h  |  6 +++
 drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++-
 5 files changed, 142 insertions(+), 37 deletions(-)

-- 
2.9.3

[PATCH] drm: Fix error handling in drm_mm eviction kselftest

[Chris Wilson]

drivers/gpu/drm/selftests/test-drm_mm.c:1277 evict_everything()
warn: calling list_del() inside list_for_each

The list_del() inside the error handling in the eviction loop is
overkill. We have to undo the eviction scan to return the drm_mm back to
a recoverable state, so have to iterate over the full list, but we only
want to report the error once and once we have an error we can return
early.

Reported-by: Dan Carpenter 
Fixes: 560b32842912 ("drm: kselftest for drm_mm and eviction")
Signed-off-by: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Daniel Vetter 
---
 drivers/gpu/drm/selftests/test-drm_mm.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/selftests/test-drm_mm.c 
b/drivers/gpu/drm/selftests/test-drm_mm.c
index ab091042f317..1e71bc182ca9 100644
--- a/drivers/gpu/drm/selftests/test-drm_mm.c
+++ b/drivers/gpu/drm/selftests/test-drm_mm.c
@@ -1272,13 +1272,19 @@ static bool evict_everything(struct drm_mm *mm,
if (drm_mm_scan_add_block(, >node))
break;
}
+
+   err = 0;
list_for_each_entry(e, _list, link) {
if (!drm_mm_scan_remove_block(, >node)) {
-   pr_err("Node %lld not marked for eviction!\n",
-  e->node.start);
-   list_del(>link);
+   if (!err) {
+   pr_err("Node %lld not marked for eviction!\n",
+  e->node.start);
+   err = -EINVAL;
+   }
}
}
+   if (err)
+   return false;

list_for_each_entry(e, _list, link)
drm_mm_remove_node(>node);
-- 
2.11.0

[bug report] drm: kselftest for drm_mm and eviction

[Chris Wilson]

On Tue, Jan 10, 2017 at 05:17:46PM +0300, Dan Carpenter wrote:
> Hello Chris Wilson,
> 
> The patch 560b32842912: "drm: kselftest for drm_mm and eviction" from
> Dec 22, 2016, leads to the following static checker warning:
> 
>   drivers/gpu/drm/selftests/test-drm_mm.c:1277 evict_everything()
>   warn: calling list_del() inside list_for_each
> 
> drivers/gpu/drm/selftests/test-drm_mm.c
>   1260  static bool evict_everything(struct drm_mm *mm,
>   1261   unsigned int total_size,
>   1262   struct evict_node *nodes)
>   1263  {
>   1264  struct drm_mm_scan scan;
>   1265  LIST_HEAD(evict_list);
>   1266  struct evict_node *e;
>   1267  unsigned int n;
>   1268  int err;
>   1269  
>   1270  drm_mm_scan_init(, mm, total_size, 0, 0, 0);
>   1271  for (n = 0; n < total_size; n++) {
>   1272  e = [n];
>   1273  list_add(>link, _list);
>   1274  if (drm_mm_scan_add_block(, >node))
>   1275  break;
>   1276  }
>   1277  list_for_each_entry(e, _list, link) {
>   1278  if (!drm_mm_scan_remove_block(, >node)) {
>   1279  pr_err("Node %lld not marked for eviction!\n",
>   1280 e->node.start);
>   1281  list_del(>link);
> 
> Probably this should be using list_for_each_entry_safe(), I guess?

It can be a return false; (or something along those lines) since after
finding the first error, there are likely plenty more.

Thanks for catching this.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[GIT PULL] omapdrm changes for 4.11

[Tomi Valkeinen]

Hi Dave,

Looks like there's a trivial conflict with the latest drm-next and the
pull request below. It can be solved by just deleting the dev_load and
dev_unload functions.

 Tomi

On 09/01/17 11:45, Tomi Valkeinen wrote:
> Hi Dave,
> 
> Please pull omapdrm changes for 4.11.
> 
>  Tomi
> 
> The following changes since commit 2cf026ae85c42f253feb9f420d1b4bc99bd5503d:
> 
>   Merge branch 'linux-4.10' of git://github.com/skeggsb/linux into drm-next 
> (2016-12-13 14:29:05 +1000)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git 
> tags/omapdrm-4.11
> 
> for you to fetch changes up to 42f7f3c4811b3149253ecf2e133832c969884466:
> 
>   drm/omap: panel-sony-acx565akm.c: Add MODULE_ALIAS (2017-01-04 14:08:14 
> +0200)
> 
> 
> omapdrm changes for 4.11
> 
> The main change here is the IRQ code cleanup, which gives us properly working
> vblank counts and timestamps. We also get much less calls to runtime PM gets &
> puts.
> 
> 
> H. Nikolaus Schaller (1):
>   drm/omap: dsi: fix compile errors when enabling debug prints
> 
> Jarkko Nikula (1):
>   drm/omap: panel-sony-acx565akm.c: Add MODULE_ALIAS
> 
> Laurent Pinchart (24):
>   drm: Kbuild: add omap_drm.h to the installed headers
>   drm: omapdrm: fb: Limit number of planes per framebuffer to two
>   drm: omapdrm: fb: Use format information provided by the DRM core
>   drm: omapdrm: fb: Simplify objects lookup when creating framebuffer
>   drm: omapdrm: fb: Simplify mode command checks when creating framebuffer
>   drm: omapdrm: fb: Turn framebuffer creation error messages into debug
>   drm: omapdrm: Handle FIFO underflow IRQs internally
>   drm: omapdrm: Handle CRTC error IRQs directly
>   drm: omapdrm: Handle OCP error IRQ directly
>   drm: omapdrm: Replace DSS manager state check with omapdrm CRTC state
>   drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs
>   drm: omapdrm: Check the CRTC software state at enable/disable time
>   drm: omapdrm: Prevent processing the same event multiple times
>   drm: omapdrm: Use a spinlock to protect the CRTC pending flag
>   drm: omapdrm: Keep vblank interrupt enabled while CRTC is active
>   drm: omapdrm: Don't expose the omap_irq_(un)register() functions
>   drm: omapdrm: Remove unused parameter from omap_drm_irq handler
>   drm: omapdrm: Don't call DISPC power handling in IRQ wait functions
>   drm: omapdrm: Inline the pipe2vbl function
>   drm: omapdrm: Simplify IRQ wait implementation
>   drm: omapdrm: Remove global variables
>   drm: omapdrm: Use sizeof(*var) instead of sizeof(type) for structures
>   drm: Move vblank cleanup from unregister to release
>   drm: omapdrm: Perform initialization/cleanup at probe/remove time
> 
> Tomi Valkeinen (1):
>   Merge omapdrm work from Laurent
> 
>  drivers/gpu/drm/drm_drv.c  |   4 +-
>  drivers/gpu/drm/omapdrm/displays/panel-dsi-cm.c|   2 +-
>  .../drm/omapdrm/displays/panel-sony-acx565akm.c|   1 +
>  drivers/gpu/drm/omapdrm/dss/dispc.c|  27 ++-
>  drivers/gpu/drm/omapdrm/dss/dsi.c  |  18 +-
>  drivers/gpu/drm/omapdrm/dss/omapdss-boot-init.c|   3 +-
>  drivers/gpu/drm/omapdrm/dss/omapdss.h  |   1 -
>  drivers/gpu/drm/omapdrm/omap_connector.c   |   6 +-
>  drivers/gpu/drm/omapdrm/omap_crtc.c| 154 ++---
>  drivers/gpu/drm/omapdrm/omap_dmm_tiler.c   |   4 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c | 218 +--
>  drivers/gpu/drm/omapdrm/omap_drv.h |  51 +
>  drivers/gpu/drm/omapdrm/omap_encoder.c |   2 +-
>  drivers/gpu/drm/omapdrm/omap_fb.c  | 164 +++---
>  drivers/gpu/drm/omapdrm/omap_irq.c | 242 
> +++--
>  drivers/gpu/drm/omapdrm/omap_plane.c   |  24 --
>  include/uapi/drm/Kbuild|   1 +
>  17 files changed, 441 insertions(+), 481 deletions(-)
> 

-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/e49b7e23/attachment.sig>

DRM range manger test failures

[Chris Wilson]

On Tue, Jan 10, 2017 at 02:51:17PM +0100, Daniel Vetter wrote:
> On Tue, Jan 10, 2017 at 12:45:04PM +, Chris Wilson wrote:
> > On Tue, Jan 10, 2017 at 01:41:54PM +0100, Geert Uytterhoeven wrote:
> > > Hi Chris, Laurent,
> > > 
> > > On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
> > > manager selftests fail with:
> > > 
> > > drm_mm: Testing DRM range manger (struct drm_mm), with
> > > random_seed=0x83a9b910 max_iterations=8192 max_prime=128
> > > drm_mm: igt_sanitycheck - ok!
> > > drm_mm: node has wrong color, found 0, expected 1
> > > drm_mm: default insert failed, size 1 step 1
> > > 
> > > I have no idea what this means.
> > 
> > The existing code has bugs. We got as far as applying the testcases to
> > demonstrate the bugs, the actual fix is still queued.
> 
> Hm, I thought I merged them all. What's missing?

The actual fix required some coordination between drm-misc and i915
trees, so was delayed. (Only because I wanted to remove repetition of
the bug from i915 first.)
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm: exynos: Add runtime PM support to MIC driver

[Marek Szyprowski]

This patch adds pm_runtime_get/put calls to notify device core when MIC
device is really in use. This is needed to let power domain with this
device to be turned off when display is turned off.

Signed-off-by: Marek Szyprowski 
---
 drivers/gpu/drm/exynos/exynos_drm_mic.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_mic.c 
b/drivers/gpu/drm/exynos/exynos_drm_mic.c
index a0def0be6d65..f643c380cb9a 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_mic.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_mic.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -323,6 +324,7 @@ static void mic_post_disable(struct drm_bridge *bridge)
for (i = NUM_CLKS - 1; i > -1; i--)
clk_disable_unprepare(mic->clks[i]);

+   pm_runtime_put(mic->dev);
mic->enabled = 0;

 already_disabled:
@@ -338,6 +340,8 @@ static void mic_pre_enable(struct drm_bridge *bridge)
if (mic->enabled)
goto already_enabled;

+   pm_runtime_get_sync(mic->dev);
+
for (i = 0; i < NUM_CLKS; i++) {
ret = clk_prepare_enable(mic->clks[i]);
if (ret < 0) {
@@ -473,8 +477,18 @@ static int exynos_mic_probe(struct platform_device *pdev)

platform_set_drvdata(pdev, mic);

+   pm_runtime_enable(dev);
+
+   ret = component_add(dev, _mic_component_ops);
+   if (ret)
+   goto err_pm;
+
DRM_DEBUG_KMS("MIC has been probed\n");
-   return component_add(dev, _mic_component_ops);
+
+   return 0;
+
+err_pm:
+   pm_runtime_disable(dev);

 err:
return ret;
@@ -483,6 +497,7 @@ static int exynos_mic_probe(struct platform_device *pdev)
 static int exynos_mic_remove(struct platform_device *pdev)
 {
component_del(>dev, _mic_component_ops);
+   pm_runtime_disable(>dev);
return 0;
 }

-- 
1.9.1

[PATCH] drm: Fix broken VT switch with video=1366x768 option

[Takashi Iwai]

On Tue, 10 Jan 2017 13:41:42 +0100,
Daniel Vetter wrote:
> 
> On Tue, Jan 10, 2017 at 12:36:50PM +0100, Takashi Iwai wrote:
> > On Tue, 10 Jan 2017 12:28:36 +0100,
> > Ville Syrjälä wrote:
> > > 
> > > On Mon, Jan 09, 2017 at 03:56:14PM +0100, Takashi Iwai wrote:
> > > > I noticed that the VT switch doesn't work any longer with a Dell
> > > > laptop with 1366x768 eDP when the machine is connected with a DP
> > > > monitor.  It behaves as if VT were switched, but the graphics remain
> > > > frozen.  Actually the keyboard works, so I could switch back to VT7
> > > > again.
> > > > 
> > > > I tried to track down the problem, and encountered a long story until
> > > > we reach to this error:
> > > > 
> > > > - The machine is booted with video=1366x768 option (the distro
> > > >   installer seems to add it as default).
> > > > - Recently, drm_helper_probe_single_connector_modes() deals with
> > > >   cmdline modes, and it tries to create a new mode when no
> > > >   matching mode is found.
> > > > - The drm_mode_create_from_cmdline_mode() creates a mode based on
> > > >   either CVT of GFT according to the given cmdline mode; in our case,
> > > >   it's 1366x768.
> > > > - Since both CVT and GFT can't express the width 1366 due to
> > > >   alignment, the resultant mode becomes 1368x768, slightly larger than
> > > >   the given size.
> > > > - Later on, the atomic commit is performed, and in
> > > >   drm_atomic_check_only(), the size of each plane is checked.
> > > > - The size check of 1366x768 fails due to the above, and eventually
> > > >   the whole VT switch fails.
> > > > 
> > > > Back in the history, we've had a manual fix-up of 1368x768 in various
> > > > places via c09dedb7a50e ("drm/edid: Add a workaround for 1366x768 HD
> > > > panel"), but they have been all in drm_edid.c at probing the modes
> > > > from EDID.  For addressing the problem above, we need a similar hack
> > > > to the mode newly created from cmdline, manually adjusting the width
> > > > when the expected size is 1366 while we get 1368 instead.
> > > > 
> > > > Fixes: eaf99c749d43 ("drm: Perform cmdline mode parsing during...")
> > > > Cc: 
> > > > Signed-off-by: Takashi Iwai 
> > > > ---
> > > >  drivers/gpu/drm/drm_modes.c | 7 +++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > > index ac6a35212501..e6b19bc9021a 100644
> > > > --- a/drivers/gpu/drm/drm_modes.c
> > > > +++ b/drivers/gpu/drm/drm_modes.c
> > > > @@ -1460,6 +1460,13 @@ drm_mode_create_from_cmdline_mode(struct 
> > > > drm_device *dev,
> > > > return NULL;
> > > >  
> > > > mode->type |= DRM_MODE_TYPE_USERDEF;
> > > > +   /* fix up 1368x768: GFT/CVT can't express 1366 width due to 
> > > > alignment */
> > > > +   if (cmd->xres == 1366 && mode->hdisplay == 1368) {
> > > > +   mode->hdisplay = 1366;
> > > > +   mode->hsync_start--;
> > > > +   mode->hsync_end--;
> > > > +   drm_mode_set_name(mode);
> > > > +   }
> > > 
> > > Maybe give fixup_mode_1366x768() a drm_ prefix and make in non-static to
> > > avoid duplicating the code? And I guess move it to drm_modes.c?
> > 
> > Yes, that'll be a good cleanup.  I was afraid of symbol export, but
> > both are in the same module, so far.  I can post a follow-up once when
> > this one gets accepted.
> 
> For the follow up, pls put the decl into drm_crtc_internal.h, so that
> drivers aren't tempted to use it. That header is for module-internal
> shared code only.

Alright, thanks for information.


Takashi

DRM range manger test failures

[Geert Uytterhoeven]

Hi Chris, Laurent,

On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
manager selftests fail with:

drm_mm: Testing DRM range manger (struct drm_mm), with
random_seed=0x83a9b910 max_iterations=8192 max_prime=128
drm_mm: igt_sanitycheck - ok!
drm_mm: node has wrong color, found 0, expected 1
drm_mm: default insert failed, size 1 step 1

I have no idea what this means.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 
linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

[PATCH] drm: Fix broken VT switch with video=1366x768 option

[Daniel Vetter]

On Tue, Jan 10, 2017 at 12:36:50PM +0100, Takashi Iwai wrote:
> On Tue, 10 Jan 2017 12:28:36 +0100,
> Ville Syrjälä wrote:
> > 
> > On Mon, Jan 09, 2017 at 03:56:14PM +0100, Takashi Iwai wrote:
> > > I noticed that the VT switch doesn't work any longer with a Dell
> > > laptop with 1366x768 eDP when the machine is connected with a DP
> > > monitor.  It behaves as if VT were switched, but the graphics remain
> > > frozen.  Actually the keyboard works, so I could switch back to VT7
> > > again.
> > > 
> > > I tried to track down the problem, and encountered a long story until
> > > we reach to this error:
> > > 
> > > - The machine is booted with video=1366x768 option (the distro
> > >   installer seems to add it as default).
> > > - Recently, drm_helper_probe_single_connector_modes() deals with
> > >   cmdline modes, and it tries to create a new mode when no
> > >   matching mode is found.
> > > - The drm_mode_create_from_cmdline_mode() creates a mode based on
> > >   either CVT of GFT according to the given cmdline mode; in our case,
> > >   it's 1366x768.
> > > - Since both CVT and GFT can't express the width 1366 due to
> > >   alignment, the resultant mode becomes 1368x768, slightly larger than
> > >   the given size.
> > > - Later on, the atomic commit is performed, and in
> > >   drm_atomic_check_only(), the size of each plane is checked.
> > > - The size check of 1366x768 fails due to the above, and eventually
> > >   the whole VT switch fails.
> > > 
> > > Back in the history, we've had a manual fix-up of 1368x768 in various
> > > places via c09dedb7a50e ("drm/edid: Add a workaround for 1366x768 HD
> > > panel"), but they have been all in drm_edid.c at probing the modes
> > > from EDID.  For addressing the problem above, we need a similar hack
> > > to the mode newly created from cmdline, manually adjusting the width
> > > when the expected size is 1366 while we get 1368 instead.
> > > 
> > > Fixes: eaf99c749d43 ("drm: Perform cmdline mode parsing during...")
> > > Cc: 
> > > Signed-off-by: Takashi Iwai 
> > > ---
> > >  drivers/gpu/drm/drm_modes.c | 7 +++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > > index ac6a35212501..e6b19bc9021a 100644
> > > --- a/drivers/gpu/drm/drm_modes.c
> > > +++ b/drivers/gpu/drm/drm_modes.c
> > > @@ -1460,6 +1460,13 @@ drm_mode_create_from_cmdline_mode(struct 
> > > drm_device *dev,
> > >   return NULL;
> > >  
> > >   mode->type |= DRM_MODE_TYPE_USERDEF;
> > > + /* fix up 1368x768: GFT/CVT can't express 1366 width due to alignment */
> > > + if (cmd->xres == 1366 && mode->hdisplay == 1368) {
> > > + mode->hdisplay = 1366;
> > > + mode->hsync_start--;
> > > + mode->hsync_end--;
> > > + drm_mode_set_name(mode);
> > > + }
> > 
> > Maybe give fixup_mode_1366x768() a drm_ prefix and make in non-static to
> > avoid duplicating the code? And I guess move it to drm_modes.c?
> 
> Yes, that'll be a good cleanup.  I was afraid of symbol export, but
> both are in the same module, so far.  I can post a follow-up once when
> this one gets accepted.

For the follow up, pls put the decl into drm_crtc_internal.h, so that
drivers aren't tempted to use it. That header is for module-internal
shared code only.

> > Otherwise lgtm:
> > Reviewed-by: Ville Syrjälä 
> 
> Thanks!

Ville, can you pls also apply this one to drm-misc-fixes?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[rfc] fix output poll execute lockdep

[Daniel Vetter]

On Tue, Jan 10, 2017 at 09:46:26PM +1000, Dave Airlie wrote:
> On 10 Jan. 2017 19:50, "Daniel Vetter"  wrote:
> 
> On Tue, Jan 10, 2017 at 12:12:30PM +1000, Dave Airlie wrote:
> > On runtime resume, nouveau can try and take the mode_config
> > mutex in the poll reenable, however a poll can race with this,
> > and take the mutex and get stuck waiting for the runtime to
> > finish completion. These two patches allow the driver to
> > get hooked in before the mutex is taken.
> >
> > I think radeon/amdgpu will also need similar patches to nouveau.
> >
> > I found this while trying to track down a runtime regression
> > on W541 laptop, I'm not sure I found what the reporter was seeing yet.
> 
> Hm, we fixed this by doing the rpm stuff always within any of the big core
> locks. And I think that's the much more reasonable design, at least as
> soon as you have more fine-grained locking.
> 
> But maybe there's a cheap way out - why does nouveau take the modeset
> lock? Ime you can remove a lot of the kms locking super easily because
> it's all cargo-culted. The last hold-out was connector_list walking, but
> that's fixed now and also doesn't need any of the heavy-weight kms locks
> anymore.
> 
> 
> Reenable polling takes the lock.

I think with the revamped connector_locking we can nuke that. All the
other stuff checked (poll_enabled and similar) are either intentionally
racy, or synchronized through the right order of the driver setup/teardown
code.

I'll be typing some patches.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2] drm: get fbdev size from cmdline mode if it exists

[Vincent ABRIOU]

On 01/10/2017 12:39 PM, Daniel Vetter wrote:
> On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
>> In case no connector is found while creating the fbdev, gives the
>> possibility to specify the default fbdev size by firstly checking if the
>> command line is defining a preferred mode. Else go into fallback and set
>> 1024x768 fbdev size as it was already done.
>>
>> Cc: Tomi Valkeinen 
>> Signed-off-by: Vincent Abriou 
>
> btw on all this there's also the possible solution to delay setup of the
> fbdev until the first connector switches to connected, and then only
> allocting the fb and doing the setup. Tegra has that, and Thierry did some
> patches to move that logic into the fb helpers. But there's some locking
> issues that need to be fixed first, hence why those patches haven't landed
> yet.
>
> But if you never probe the right mode, this here sounds like a good idea
> too.
> -Daniel

The early creation of fbdev is useful for userland system. If fbdev 
creation is delayed until first connector is connected, userland systems 
startup could fails (like Android that check fbdev availability at startup).

Today if no connector is connected, a default 1024x768 fbdev is created 
but it does not necessarily match the targeted CRTC size. When the 
connector is connected, fbdev is not reconfigured with the targeted CRTC 
size and it is anyway too late for the userland to take into account new 
fbdev size.
Reading the cmdline is an easy way to solve this.

Regards,
Vincent

>> ---
>> Patch v2:
>>  add a break in the connector for loop when a first cmdline mode is found
>>
>>  drivers/gpu/drm/drm_fb_helper.c | 34 +-
>>  1 file changed, 29 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c 
>> b/drivers/gpu/drm/drm_fb_helper.c
>> index 0ab6aaa..b38285f 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1526,6 +1526,7 @@ static int drm_fb_helper_single_fb_probe(struct 
>> drm_fb_helper *fb_helper,
>>  }
>>
>>  crtc_count = 0;
>> +
>>  for (i = 0; i < fb_helper->crtc_count; i++) {
>>  struct drm_display_mode *desired_mode;
>>  struct drm_mode_set *mode_set;
>> @@ -1570,11 +1571,34 @@ static int drm_fb_helper_single_fb_probe(struct 
>> drm_fb_helper *fb_helper,
>>  }
>>
>>  if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
>> -/* hmm everyone went away - assume VGA cable just fell out
>> -   and will come back later. */
>> -DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
>> -sizes.fb_width = sizes.surface_width = 1024;
>> -sizes.fb_height = sizes.surface_height = 768;
>> +struct drm_display_mode *mode = NULL;
>> +/* hmm everyone went away - assume cable just fell out and will
>> + * come back later.
>> + * Get fb size from command line mode (if existing) else fb size
>> + * is set to 1024x768
>> + */
>> +for (i = 0; i < fb_helper->connector_count; i++) {
>> +struct drm_fb_helper_connector *fb_helper_conn;
>> +
>> +fb_helper_conn = fb_helper->connector_info[i];
>> +mode = drm_pick_cmdline_mode(fb_helper_conn);
>> +if (mode)
>> +break;
>> +}
>> +
>> +if (mode) {
>> +sizes.fb_width = mode->hdisplay;
>> +sizes.fb_height = mode->vdisplay;
>> +DRM_INFO("Cannot find any crtc or sizes - use cmdline 
>> %dx%d\n",
>> + sizes.fb_width, sizes.fb_height);
>> +} else {
>> +sizes.fb_width = 1024;
>> +sizes.fb_height = 768;
>> +DRM_INFO("Cannot find any crtc or sizes - going 
>> 1024x768\n");
>> +}
>> +
>> +sizes.surface_width = sizes.fb_width;
>> +sizes.surface_height = sizes.fb_height;
>>  }
>>
>>  /* push down into drivers */
>> --
>> 2.7.4
>>
>> ___
>> dri-devel mailing list
>> dri-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>

[PATCH] drm: Fix broken VT switch with video=1366x768 option

[Ville Syrjälä]

On Mon, Jan 09, 2017 at 03:56:14PM +0100, Takashi Iwai wrote:
> I noticed that the VT switch doesn't work any longer with a Dell
> laptop with 1366x768 eDP when the machine is connected with a DP
> monitor.  It behaves as if VT were switched, but the graphics remain
> frozen.  Actually the keyboard works, so I could switch back to VT7
> again.
> 
> I tried to track down the problem, and encountered a long story until
> we reach to this error:
> 
> - The machine is booted with video=1366x768 option (the distro
>   installer seems to add it as default).
> - Recently, drm_helper_probe_single_connector_modes() deals with
>   cmdline modes, and it tries to create a new mode when no
>   matching mode is found.
> - The drm_mode_create_from_cmdline_mode() creates a mode based on
>   either CVT of GFT according to the given cmdline mode; in our case,
>   it's 1366x768.
> - Since both CVT and GFT can't express the width 1366 due to
>   alignment, the resultant mode becomes 1368x768, slightly larger than
>   the given size.
> - Later on, the atomic commit is performed, and in
>   drm_atomic_check_only(), the size of each plane is checked.
> - The size check of 1366x768 fails due to the above, and eventually
>   the whole VT switch fails.
> 
> Back in the history, we've had a manual fix-up of 1368x768 in various
> places via c09dedb7a50e ("drm/edid: Add a workaround for 1366x768 HD
> panel"), but they have been all in drm_edid.c at probing the modes
> from EDID.  For addressing the problem above, we need a similar hack
> to the mode newly created from cmdline, manually adjusting the width
> when the expected size is 1366 while we get 1368 instead.
> 
> Fixes: eaf99c749d43 ("drm: Perform cmdline mode parsing during...")
> Cc: 
> Signed-off-by: Takashi Iwai 
> ---
>  drivers/gpu/drm/drm_modes.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index ac6a35212501..e6b19bc9021a 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -1460,6 +1460,13 @@ drm_mode_create_from_cmdline_mode(struct drm_device 
> *dev,
>   return NULL;
>  
>   mode->type |= DRM_MODE_TYPE_USERDEF;
> + /* fix up 1368x768: GFT/CVT can't express 1366 width due to alignment */
> + if (cmd->xres == 1366 && mode->hdisplay == 1368) {
> + mode->hdisplay = 1366;
> + mode->hsync_start--;
> + mode->hsync_end--;
> + drm_mode_set_name(mode);
> + }

Maybe give fixup_mode_1366x768() a drm_ prefix and make in non-static to
avoid duplicating the code? And I guess move it to drm_modes.c?

Otherwise lgtm:
Reviewed-by: Ville Syrjälä 

>   drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
>   return mode;
>  }
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC

[RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB

[Ville Syrjälä]

On Thu, Jan 05, 2017 at 02:46:06PM +, Jose Abreu wrote:
> Hi Ville,
> 
> 
> On 05-01-2017 11:46, Ville Syrjälä wrote:
> > On Thu, Jan 05, 2017 at 10:07:45AM +, Jose Abreu wrote:
> >> Hi Ville,
> >>
> >>
> >> On 04-01-2017 16:36, Ville Syrjälä wrote:
> >>> On Wed, Jan 04, 2017 at 04:15:01PM +, Jose Abreu wrote:
> >>  
> >> [snip]
> >>
> > Why does userspace need to know this? My thinking has been that the
> > driver would do the right thing automagically.
> >
> > We do probably want some kind of output colorspace property to allow the
> > user to select between RGB vs. YCbCr etc. But I think even with that we
> > should still allow the driver to automagically select YCbCr 4:2:0 output
> > since that's the only way the mode will work.
>  I agree. When only 4:2:0 is supported there is no need to expose
>  the flag to userspace. How shall then I signal drivers for this
>  4:2:0'only sampling mode?
> 
>  So, for the remaining modes, you propose a new field in the mode
>  structure called 'colorspace' which contains the list of
>  supported sampling modes for the given mode? I think it would be
>  a nice addition. This way if a mode supports only RGB we only
>  passed RGB flag; if 4:2:0 was also supported we passed the 4:2:0
>  flag, ... And then user could select. We also have to inform user
>  which one is being actually used.
> >>> IIRC there aren't any "RGB only" modes or anything like that. So
> >>> YCbCr 4:2:0 is the special case here. We could just add something to the
> >>> mode struct for it, or do we already have some other flags thing that's
> >>> not exposed to userspace? And I guess drivers should be able to opt into
> >>> supporting these 4:2:0 modes in similar way they opt into
> >>> interlaced/stereo/whatever.
> >> I mean, if a source EDID does not declare support for YCbCr modes
> >> (4:2:2 and 4:4:4 [i think they have to be both supported if sink
> >> supports != RGB]) then only RGB can be used. Or is any YCbCr that
> >> is pre-required? Still, I see your point. When EDID declares
> >> support for YCbCr then all modes can use it, and not only some of
> >> them.
> >>
> >> I think for stereo modes the flags can be opt in/out in userspace
> >> exposing. There is a function called
> >> drm_mode_expose_to_userspace() which only exposes stereo modes if
> >> user asks to. We could do something similar for 4:2:0 modes (or
> >> even for all pixel encoding). i.e. expose which encoding can be
> >> used in current video mode. What do you think?
> >>
> >> About drivers opting in for 4:2:0 modes, then you propose a new
> >> field in drm_connector (called for example ycbcr_420_allowed)
> >> which only does the parsing of the 4:2:0 modes and adds them to
> >> the list when set to true?
> > Thinking a bit more about this, we do have a slight problem with not
> > exposing this information to userspace. Namely we can't actually tell
> > whether any user provided mode would need to output as 4:2:0 or not.
> > With the new flag userspace could at least inherit that from the kernel
> > and pass it back in. But still, expanding the uapi for something like
> > this feels quite wrong to me. Can we simply look at a particular user
> > supplied mode and tell whether it needs to be output as 4:2:0 (based
> > on eg. dotclock)?
> >
> 
> The pixel clock rate is half of the TMDS character rate in 4:2:0
> (in 24 bit), but for example in deep color 48 bit it will be the
> same rate. There is also a reduction to half of htotal, hactive,
> hblank, hfront, hsync and hback but I don't think it's a good
> solution to guide us from there.

I was asking if we can look at a specific modeline and whether we
can tell from that if we would need to output it as 4:2:0.

> Why does it feel wrong to you
> expanding the uapi?

Because it requires changing every single userspace kms client. And
it's not something userspace should have to worry about.

> 
> I think its important to say that the chosen colorspace can
> improve performance in systems: for example, as I said, 4:2:0
> 24-bit uses half the rate that RGB 24-bit uses so we have less
> trafic in the bus. I am recently working with a FPGA connected
> trough pcie and I can definitely say that this is true. But, as
> expected, less traffic means less quality in final image so its
> not a matter of letting kernel decide, I think its a matter of
> user choosing between performance vs. quality.

Image quality control for userspace is a much bigger topic. And
something we have no real precedent for at the moment (apart from 
user choosing a different fb pixel format).

The performance arument is very hardware dependent, and not really
all that relevant IMO. If the user wants the big mode they either
get it or not depending on whether the system can deliver.

-- 
Ville Syrjälä
Intel OTC

[PATCH 06/10] drm/i915/psr: set CHICKEN_TRANS for psr2

[Ville Syrjälä]

On Mon, Jan 09, 2017 at 06:38:15PM +0530, vathsala nagaraju wrote:
> As per bpsec, CHICKEN_TRANS_EDP bit 12 ,15 must be programmed in
> psr2 enable sequence.
> Program Transcoder EDP VSC DIP header with a valid setting for PSR2
> and Set CHICKEN_TRANS_EDP(0x420cc) bit 12 for programmable header
> packet.
> Set CHICKEN_TRANS_EDP(0x420cc) bit 15 if Y coordinate is supported
> 
> v2: (Rodrigo)
> - move CHICKEN_TRANS_EDP bit set logic right after setup_vsc
> 
> v3:(Rodrigo)
> - initialize chicken_trans to CHICKEN_TRANS_BIT12 instead of 0
> 
> v4:(chris wilson)
> - use BIT(12), remove CHICKEN_TRANS_BIT12
> - remove unnecessary comments
> - update commit message
> 
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: vathsala nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  | 5 +
>  drivers/gpu/drm/i915/intel_psr.c | 5 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7830e6e..3299e01 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6449,6 +6449,11 @@ enum {
>  #define  BDW_DPRS_MASK_VBLANK_SRD(1 << 0)
>  #define CHICKEN_PIPESL_1(pipe) _MMIO_PIPE(pipe, _CHICKEN_PIPESL_1_A, 
> _CHICKEN_PIPESL_1_B)
>  
> +#define CHICKEN_TRANS_A 0x420c0
> +#define CHICKEN_TRANS_B 0x420c4
> +#define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, 
> CHICKEN_TRANS_B)
> +#define TRANS_EDP  3
> +
>  #define DISP_ARB_CTL _MMIO(0x45000)
>  #define  DISP_FBC_MEMORY_WAKE(1<<31)
>  #define  DISP_TILE_SURFACE_SWIZZLING (1<<13)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c 
> b/drivers/gpu/drm/i915/intel_psr.c
> index b28891b..b1686c2 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -475,6 +475,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   struct drm_device *dev = intel_dig_port->base.base.dev;
>   struct drm_i915_private *dev_priv = to_i915(dev);
>   struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);
> + u32 chicken = 0;
>  
>   if (!HAS_PSR(dev_priv)) {
>   DRM_DEBUG_KMS("PSR not supported on this platform\n");
> @@ -505,6 +506,10 @@ void intel_psr_enable(struct intel_dp *intel_dp)
>   dev_priv->psr.psr2_support = false;
>   else
>   skl_psr_setup_su_vsc(intel_dp);
> + chicken = BIT(12);
> + if (dev_priv->psr.y_cord_support)
> + chicken |= BIT(15);

In cases like this I think the better option is to invent a decent
name for the bit. Otherwise no one can figure out what the code does
without consulting the spec.

> + I915_WRITE(CHICKEN_TRANS(TRANS_EDP), chicken);
>   } else {
>   /* set up vsc header for psr1 */
>   hsw_psr_setup_vsc(intel_dp);
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

[PATCH] drm: disable deep color when EDID violates spec

[Ville Syrjälä]

On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
> As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
> greater than 24 bits per pixel. The spec explicitly states, "All Deep
> Color modes are optional though if an HDMI Source or Sink supports any
> Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
> Requirements).
> 
> I came across a monitor (Acer X233H) that supplies an illegal EDID where
> DC_30bit is set and DC_36bit is not set. The end result is badly garbled
> output because the driver is sending 36BPP when the monitor can't handle
> it.
> 
> Much of the intel hardware is incapable of operating at any
> bit-per-component setting outside of 8 or 12, and the spec seems to
> imply that if any deep color support is found, then it is a safe
> assumption to operate at 12.
> 
> This patch ensures that the EDID is within the spec (specifically, that
> DC_36bit is set) before committing to going forward with any deep
> colors.  There was already a check for this EDID inconsistency, but it
> resulted only in a warning and did not fall-back to safer settings.
> 
> CC: Ville Syrjälä 
> Signed-off-by: Nicholas Sielicki 
> ---
>  drivers/gpu/drm/drm_edid.c | 35 +++
>  1 file changed, 23 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 336be31ff3de..42ce3f54d2dc 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3772,30 +3772,34 @@ static void drm_parse_hdmi_deep_color_info(struct 
> drm_connector *connector,
>  {
>   struct drm_display_info *info = >display_info;
>   unsigned int dc_bpc = 0;
> + u8 modes = 0;
>  
>   /* HDMI supports at least 8 bpc */
>   info->bpc = 8;
>  
> + /* Ensure all DC modes are unset if we return early */
> + info->edid_hdmi_dc_modes = 0;

Clearing this in drm_add_display_info() should be sufficient since
this guy doesn't get called from anywhere else. So this part could
be droppped.

Otherwise this feels like a decent way to handle the problem. We
could of course try to use the 10bpc (or whatever) deep color modes
the sink claims to support, but given that the people designing the
thing didn't bother reading the spec, it seems safer to just disable
deep color support entirely.

Reviewed-by: Ville Syrjälä 

> +
>   if (cea_db_payload_len(hdmi) < 6)
>   return;
>  
>   if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
>   dc_bpc = 10;
> - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
> + modes |= DRM_EDID_HDMI_DC_30;
>   DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
> connector->name);
>   }
>  
>   if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
>   dc_bpc = 12;
> - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
> + modes |= DRM_EDID_HDMI_DC_36;
>   DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
> connector->name);
>   }
>  
>   if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
>   dc_bpc = 16;
> - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
> + modes |= DRM_EDID_HDMI_DC_48;
>   DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
> connector->name);
>   }
> @@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_info(struct 
> drm_connector *connector,
>   return;
>   }
>  
> + /*
> +  * All deep color modes are optional, but if a sink supports any deep
> +  * color mode, it must support 36-bit mode. If this is found not
> +  * to be the case, sink is in violation of HDMI 1.3 / 1.4 spec and it
> +  * is prudent to disable all deep color modes. Return here before
> +  * committing bpc and edid_hdmi_dc_modes.
> +  */
> + if (!(modes & DRM_EDID_HDMI_DC_36)) {
> + DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
> +   connector->name);
> + return;
> + }
> +
> +
>   DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
> connector->name, dc_bpc);
>   info->bpc = dc_bpc;
> + info->edid_hdmi_dc_modes = modes;
>  
>   /*
>* Deep color support mandates RGB444 support for all video
> @@ -3823,15 +3842,6 @@ static void drm_parse_hdmi_deep_color_info(struct 
> drm_connector *connector,
>   DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",
> connector->name);
>   }
> -
> - /*
> -  * Spec says that if any deep color mode is supported at all,
> -  * then deep color 36 bit must be supported.
> -  */
> - if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
> - DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
> -   connector->name);
> - }
>  }
>  
>  static void
> @@ -3895,6 +3905,7 @@ static void 

[Bug 99343] dEQP-GLES31.functional.shaders.builtin_functions.precision.{min, max}.highp_compute.scalar failures

[bugzilla-dae...@freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=99343

Bug ID: 99343
   Summary: dEQP-GLES31.functional.shaders.builtin_functions.preci
sion.{min,max}.highp_compute.scalar failures
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: samuel.pitoiset at gmail.com
QA Contact: dri-devel at lists.freedesktop.org

Hi,

I have noticed that the following dEQP GLES31 tests fail with latest mesa/llvm:

dEQP-GLES31.functional.shaders.builtin_functions.precision.min.highp_compute.scalar
dEQP-GLES31.functional.shaders.builtin_functions.precision.max.highp_compute.scalar

The same tests with lowp instead of highp work fine. Note that it's unrelated
to compute because they also fail with vertex shaders.

After glancing at the code, it seems like enabling denorms for 32-bit floats
fix them. But it's currently only enabled for 64-bit and 16-bit floats for some
reasons.

Marek, Nicolai, any thoughts about that?

I didn't try if these tests pass with Catalyst though, but they do at least on
NVIDIA hw.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/daebc924/attachment-0001.html>

DRM range manger test failures

[Chris Wilson]

On Tue, Jan 10, 2017 at 01:41:54PM +0100, Geert Uytterhoeven wrote:
> Hi Chris, Laurent,
> 
> On R-Car Gen2 (Koelsch) and Gen3 (Salvator-X), the new DRM range
> manager selftests fail with:
> 
> drm_mm: Testing DRM range manger (struct drm_mm), with
> random_seed=0x83a9b910 max_iterations=8192 max_prime=128
> drm_mm: igt_sanitycheck - ok!
> drm_mm: node has wrong color, found 0, expected 1
> drm_mm: default insert failed, size 1 step 1
> 
> I have no idea what this means.

The existing code has bugs. We got as far as applying the testcases to
demonstrate the bugs, the actual fix is still queued.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH v2] drm: get fbdev size from cmdline mode if it exists

[Daniel Vetter]

On Tue, Jan 10, 2017 at 12:21:09PM +0100, Vincent Abriou wrote:
> In case no connector is found while creating the fbdev, gives the
> possibility to specify the default fbdev size by firstly checking if the
> command line is defining a preferred mode. Else go into fallback and set
> 1024x768 fbdev size as it was already done.
> 
> Cc: Tomi Valkeinen 
> Signed-off-by: Vincent Abriou 

btw on all this there's also the possible solution to delay setup of the
fbdev until the first connector switches to connected, and then only
allocting the fb and doing the setup. Tegra has that, and Thierry did some
patches to move that logic into the fb helpers. But there's some locking
issues that need to be fixed first, hence why those patches haven't landed
yet.

But if you never probe the right mode, this here sounds like a good idea
too.
-Daniel
> ---
> Patch v2:
>  add a break in the connector for loop when a first cmdline mode is found
> 
>  drivers/gpu/drm/drm_fb_helper.c | 34 +-
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 0ab6aaa..b38285f 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1526,6 +1526,7 @@ static int drm_fb_helper_single_fb_probe(struct 
> drm_fb_helper *fb_helper,
>   }
>  
>   crtc_count = 0;
> +
>   for (i = 0; i < fb_helper->crtc_count; i++) {
>   struct drm_display_mode *desired_mode;
>   struct drm_mode_set *mode_set;
> @@ -1570,11 +1571,34 @@ static int drm_fb_helper_single_fb_probe(struct 
> drm_fb_helper *fb_helper,
>   }
>  
>   if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
> - /* hmm everyone went away - assume VGA cable just fell out
> -and will come back later. */
> - DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
> - sizes.fb_width = sizes.surface_width = 1024;
> - sizes.fb_height = sizes.surface_height = 768;
> + struct drm_display_mode *mode = NULL;
> + /* hmm everyone went away - assume cable just fell out and will
> +  * come back later.
> +  * Get fb size from command line mode (if existing) else fb size
> +  * is set to 1024x768
> +  */
> + for (i = 0; i < fb_helper->connector_count; i++) {
> + struct drm_fb_helper_connector *fb_helper_conn;
> +
> + fb_helper_conn = fb_helper->connector_info[i];
> + mode = drm_pick_cmdline_mode(fb_helper_conn);
> + if (mode)
> + break;
> + }
> +
> + if (mode) {
> + sizes.fb_width = mode->hdisplay;
> + sizes.fb_height = mode->vdisplay;
> + DRM_INFO("Cannot find any crtc or sizes - use cmdline 
> %dx%d\n",
> +  sizes.fb_width, sizes.fb_height);
> + } else {
> + sizes.fb_width = 1024;
> + sizes.fb_height = 768;
> + DRM_INFO("Cannot find any crtc or sizes - going 
> 1024x768\n");
> + }
> +
> + sizes.surface_width = sizes.fb_width;
> + sizes.surface_height = sizes.fb_height;
>   }
>  
>   /* push down into drivers */
> -- 
> 2.7.4
> 
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH] drm: Fix broken VT switch with video=1366x768 option

[Takashi Iwai]

On Tue, 10 Jan 2017 12:28:36 +0100,
Ville Syrjälä wrote:
> 
> On Mon, Jan 09, 2017 at 03:56:14PM +0100, Takashi Iwai wrote:
> > I noticed that the VT switch doesn't work any longer with a Dell
> > laptop with 1366x768 eDP when the machine is connected with a DP
> > monitor.  It behaves as if VT were switched, but the graphics remain
> > frozen.  Actually the keyboard works, so I could switch back to VT7
> > again.
> > 
> > I tried to track down the problem, and encountered a long story until
> > we reach to this error:
> > 
> > - The machine is booted with video=1366x768 option (the distro
> >   installer seems to add it as default).
> > - Recently, drm_helper_probe_single_connector_modes() deals with
> >   cmdline modes, and it tries to create a new mode when no
> >   matching mode is found.
> > - The drm_mode_create_from_cmdline_mode() creates a mode based on
> >   either CVT of GFT according to the given cmdline mode; in our case,
> >   it's 1366x768.
> > - Since both CVT and GFT can't express the width 1366 due to
> >   alignment, the resultant mode becomes 1368x768, slightly larger than
> >   the given size.
> > - Later on, the atomic commit is performed, and in
> >   drm_atomic_check_only(), the size of each plane is checked.
> > - The size check of 1366x768 fails due to the above, and eventually
> >   the whole VT switch fails.
> > 
> > Back in the history, we've had a manual fix-up of 1368x768 in various
> > places via c09dedb7a50e ("drm/edid: Add a workaround for 1366x768 HD
> > panel"), but they have been all in drm_edid.c at probing the modes
> > from EDID.  For addressing the problem above, we need a similar hack
> > to the mode newly created from cmdline, manually adjusting the width
> > when the expected size is 1366 while we get 1368 instead.
> > 
> > Fixes: eaf99c749d43 ("drm: Perform cmdline mode parsing during...")
> > Cc: 
> > Signed-off-by: Takashi Iwai 
> > ---
> >  drivers/gpu/drm/drm_modes.c | 7 +++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> > index ac6a35212501..e6b19bc9021a 100644
> > --- a/drivers/gpu/drm/drm_modes.c
> > +++ b/drivers/gpu/drm/drm_modes.c
> > @@ -1460,6 +1460,13 @@ drm_mode_create_from_cmdline_mode(struct drm_device 
> > *dev,
> > return NULL;
> >  
> > mode->type |= DRM_MODE_TYPE_USERDEF;
> > +   /* fix up 1368x768: GFT/CVT can't express 1366 width due to alignment */
> > +   if (cmd->xres == 1366 && mode->hdisplay == 1368) {
> > +   mode->hdisplay = 1366;
> > +   mode->hsync_start--;
> > +   mode->hsync_end--;
> > +   drm_mode_set_name(mode);
> > +   }
> 
> Maybe give fixup_mode_1366x768() a drm_ prefix and make in non-static to
> avoid duplicating the code? And I guess move it to drm_modes.c?

Yes, that'll be a good cleanup.  I was afraid of symbol export, but
both are in the same module, so far.  I can post a follow-up once when
this one gets accepted.

> 
> Otherwise lgtm:
> Reviewed-by: Ville Syrjälä 

Thanks!


Takashi

> > drm_mode_set_crtcinfo(mode, CRTC_INTERLACE_HALVE_V);
> > return mode;
> >  }
> > -- 
> > 2.11.0
> 
> -- 
> Ville Syrjälä
> Intel OTC
> 

[PATCH] drm: disable deep color when EDID violates spec

[Ernst Sjöstrand]

.
> > +  */
> > + if (!(modes & DRM_EDID_HDMI_DC_36)) {
> > + DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
> > +   connector->name);
> > + return;
> > + }
> > +
> > +
> >   DRM_DEBUG("%s: Assigning HDMI sink color depth as %d bpc.\n",
> > connector->name, dc_bpc);
> >   info->bpc = dc_bpc;
> > + info->edid_hdmi_dc_modes = modes;
> >
> >   /*
> >* Deep color support mandates RGB444 support for all video
> > @@ -3823,15 +3842,6 @@ static void drm_parse_hdmi_deep_color_info(struct
> drm_connector *connector,
> >   DRM_DEBUG("%s: HDMI sink does YCRCB444 in deep color.\n",
> > connector->name);
> >   }
> > -
> > - /*
> > -  * Spec says that if any deep color mode is supported at all,
> > -  * then deep color 36 bit must be supported.
> > -  */
> > - if (!(hdmi[6] & DRM_EDID_HDMI_DC_36)) {
> > - DRM_DEBUG("%s: HDMI sink should do DC_36, but does not!\n",
> > -   connector->name);
> > - }
> >  }
> >
> >  static void
> > @@ -3895,6 +3905,7 @@ static void drm_add_display_info(struct
> drm_connector *connector,
> >   /* driver figures it out in this case */
> >   info->bpc = 0;
> >   info->color_formats = 0;
> > + info->edid_hdmi_dc_modes = 0;
> >   info->cea_rev = 0;
> >   info->max_tmds_clock = 0;
> >   info->dvi_dual = false;
> > --
> > 2.11.0
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20170110/22794c37/attachment-0001.html>

[PATCH] drm/i915/psr: disable psr2 for resolution greater than 32X20

[vathsala nagaraju]

PSR2 is restricted to work with panel resolutions upto 3200x2000,
move the check to intel_psr_match_conditions and fully block psr.

Cc: Rodrigo Vivi 
Cc: Jim Bride 
Suggested-by: Rodrigo Vivi 
Signed-off-by: Vathsala Nagaraju 
---
 drivers/gpu/drm/i915/intel_psr.c | 15 ---
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 6aca8ff..f2ca2a9 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -387,6 +387,13 @@ static bool intel_psr_match_conditions(struct intel_dp 
*intel_dp)
return false;
}

+   /* PSR2 is restricted to work with panel resolutions upto 3200x2000 */
+   if (intel_crtc->config->pipe_src_w > 3200 ||
+   intel_crtc->config->pipe_src_h > 2000) {
+   dev_priv->psr.psr2_support = false;
+   return false;
+   }
+
dev_priv->psr.source_ok = true;
return true;
 }
@@ -425,7 +432,6 @@ void intel_psr_enable(struct intel_dp *intel_dp)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = intel_dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
-   struct intel_crtc *crtc = to_intel_crtc(intel_dig_port->base.base.crtc);

if (!HAS_PSR(dev_priv)) {
DRM_DEBUG_KMS("PSR not supported on this platform\n");
@@ -452,12 +458,7 @@ void intel_psr_enable(struct intel_dp *intel_dp)
hsw_psr_setup_vsc(intel_dp);

if (dev_priv->psr.psr2_support) {
-   /* PSR2 is restricted to work with panel resolutions 
upto 3200x2000 */
-   if (crtc->config->pipe_src_w > 3200 ||
-   crtc->config->pipe_src_h > 2000)
-   dev_priv->psr.psr2_support = false;
-   else
-   skl_psr_setup_su_vsc(intel_dp);
+   skl_psr_setup_su_vsc(intel_dp);
}

/*
-- 
1.9.1

[PATCH] drm: disable deep color when EDID violates spec

[Alex Deucher]

On Tue, Jan 10, 2017 at 6:33 AM, Ernst Sjöstrand  wrote:
> Isn't 10bpc very common among monitors, and 12bpc very rare? Or maybe I'm
> confusing the transport layer with the presentation capabilities...?
> Here are 201 monitors that claim 10bpc:
> http://pricespy.co.uk/category.php?l=s300859434=eg_401#prodlista
>

FWIW, I've almost never seen a monitor that supports 12 bpc, but I've
see quite a few 10 bpc monitors.  I can talk to our display team to
see what other OSes do.

Alex

> Regards
> //Ernst
>
> 2017-01-10 11:52 GMT+01:00 Ville Syrjälä :
>>
>> On Thu, Jan 05, 2017 at 05:45:23PM -0600, Nicholas Sielicki wrote:
>> > As per the HDMI 1.3 and 1.4 spec, "deep color modes" are color depths
>> > greater than 24 bits per pixel. The spec explicitly states, "All Deep
>> > Color modes are optional though if an HDMI Source or Sink supports any
>> > Deep Color mode, it shall support 36-bit mode." (6.2.4 Color Depth
>> > Requirements).
>> >
>> > I came across a monitor (Acer X233H) that supplies an illegal EDID where
>> > DC_30bit is set and DC_36bit is not set. The end result is badly garbled
>> > output because the driver is sending 36BPP when the monitor can't handle
>> > it.
>> >
>> > Much of the intel hardware is incapable of operating at any
>> > bit-per-component setting outside of 8 or 12, and the spec seems to
>> > imply that if any deep color support is found, then it is a safe
>> > assumption to operate at 12.
>> >
>> > This patch ensures that the EDID is within the spec (specifically, that
>> > DC_36bit is set) before committing to going forward with any deep
>> > colors.  There was already a check for this EDID inconsistency, but it
>> > resulted only in a warning and did not fall-back to safer settings.
>> >
>> > CC: Ville Syrjälä 
>> > Signed-off-by: Nicholas Sielicki 
>> > ---
>> >  drivers/gpu/drm/drm_edid.c | 35 +++
>> >  1 file changed, 23 insertions(+), 12 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> > index 336be31ff3de..42ce3f54d2dc 100644
>> > --- a/drivers/gpu/drm/drm_edid.c
>> > +++ b/drivers/gpu/drm/drm_edid.c
>> > @@ -3772,30 +3772,34 @@ static void
>> > drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>> >  {
>> >   struct drm_display_info *info = >display_info;
>> >   unsigned int dc_bpc = 0;
>> > + u8 modes = 0;
>> >
>> >   /* HDMI supports at least 8 bpc */
>> >   info->bpc = 8;
>> >
>> > + /* Ensure all DC modes are unset if we return early */
>> > + info->edid_hdmi_dc_modes = 0;
>>
>> Clearing this in drm_add_display_info() should be sufficient since
>> this guy doesn't get called from anywhere else. So this part could
>> be droppped.
>>
>> Otherwise this feels like a decent way to handle the problem. We
>> could of course try to use the 10bpc (or whatever) deep color modes
>> the sink claims to support, but given that the people designing the
>> thing didn't bother reading the spec, it seems safer to just disable
>> deep color support entirely.
>>
>> Reviewed-by: Ville Syrjälä 
>>
>> > +
>> >   if (cea_db_payload_len(hdmi) < 6)
>> >   return;
>> >
>> >   if (hdmi[6] & DRM_EDID_HDMI_DC_30) {
>> >   dc_bpc = 10;
>> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_30;
>> > + modes |= DRM_EDID_HDMI_DC_30;
>> >   DRM_DEBUG("%s: HDMI sink does deep color 30.\n",
>> > connector->name);
>> >   }
>> >
>> >   if (hdmi[6] & DRM_EDID_HDMI_DC_36) {
>> >   dc_bpc = 12;
>> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_36;
>> > + modes |= DRM_EDID_HDMI_DC_36;
>> >   DRM_DEBUG("%s: HDMI sink does deep color 36.\n",
>> > connector->name);
>> >   }
>> >
>> >   if (hdmi[6] & DRM_EDID_HDMI_DC_48) {
>> >   dc_bpc = 16;
>> > - info->edid_hdmi_dc_modes |= DRM_EDID_HDMI_DC_48;
>> > + modes |= DRM_EDID_HDMI_DC_48;
>> >   DRM_DEBUG("%s: HDMI sink does deep color 48.\n",
>> > connector->name);
>> >   }
>> > @@ -3806,9 +3810,24 @@ static void drm_parse_hdmi_deep_color_info(struct
>> > drm_connector *connector,
>> >   return;
>> >   }
>> >
>> > + /*
>> > +  * All deep color modes are optional, but if a sink supports any
>> > deep
>> > +  * color mode, it must support 36-bit mode. If this is found not
>> > +  * to be the case, sink is in violation of HDMI 1.3 / 1.4 spec and
>> > it
>> > +  * is prudent to disable all deep color modes. Return here before
>> > +  * committing bpc and edid_hdmi_dc_modes.
>> > +  */
>> > + if (!(modes & DRM_EDID_HDMI_DC_36)) {
>> > + DRM_DEBUG("%s: HDMI sink should do DC_36, but does
>> > not!\n",
>> > +   connector->name);
>> > + return;
>> > + }
>> > +
>> > +
>> >   DRM_DEBUG("%s: Assigning HDMI 

[RFC] drm: Parse HDMI 2.0 YCbCr 4:2:0 VDB and VCB

[Jose Abreu]

Hi Ville,


On 10-01-2017 11:16, Ville Syrjälä wrote:
> On Thu, Jan 05, 2017 at 02:46:06PM +, Jose Abreu wrote:
>>

[snip]

>> The pixel clock rate is half of the TMDS character rate in 4:2:0
>> (in 24 bit), but for example in deep color 48 bit it will be the
>> same rate. There is also a reduction to half of htotal, hactive,
>> hblank, hfront, hsync and hback but I don't think it's a good
>> solution to guide us from there.
> I was asking if we can look at a specific modeline and whether we
> can tell from that if we would need to output it as 4:2:0.

Hmm, according to HDMI 2.0 spec there are no 4:2:0 only modes and
the only way to figure out if the mode is 4:2:0 only (or able) is
to parse the VCB and VBD blocks from EDID. The clock is half rate
but this is the source that has to figure it out. The mode is
still passed in a regular way (By VIC, by timing, ...).

>
>> Why does it feel wrong to you
>> expanding the uapi?
> Because it requires changing every single userspace kms client. And
> it's not something userspace should have to worry about.

I agree but, as Daniel said [1], we could make these new HDMI 2.0
features optional and only pass them to userspace if client asked
for them. What do you think?

[1]
https://lists.freedesktop.org/archives/dri-devel/2017-January/128683.html

>
>> I think its important to say that the chosen colorspace can
>> improve performance in systems: for example, as I said, 4:2:0
>> 24-bit uses half the rate that RGB 24-bit uses so we have less
>> trafic in the bus. I am recently working with a FPGA connected
>> trough pcie and I can definitely say that this is true. But, as
>> expected, less traffic means less quality in final image so its
>> not a matter of letting kernel decide, I think its a matter of
>> user choosing between performance vs. quality.
> Image quality control for userspace is a much bigger topic. And
> something we have no real precedent for at the moment (apart from 
> user choosing a different fb pixel format).
>
> The performance arument is very hardware dependent, and not really
> all that relevant IMO. If the user wants the big mode they either
> get it or not depending on whether the system can deliver.
>

Ok. But note that there is no nice way to figure this out. For
example, for a graphics card it all depends (apart from the
graphics HW) on the PCIe bus. If the bus is not free for enough
data rate then user can reach bottlenecks and not output at best
performance. If we gave user the ability to switch from, for
example, RGB to YCbCr 4:2:0 this bottleneck could be eliminated.
Unless of course we always prefer YCbCr 4:2:0, when possible. I
did this internally for bridge driver dw-hdmi. We always prefer
YCbCr over RGB when they are available. It is user transparent as
the controller does the necessary color space conversion, though,
not ideal in my opinion.

Best regards,
Jose Miguel Abreu

[PATCH v2] drm: get fbdev size from cmdline mode if it exists

[Vincent Abriou]

In case no connector is found while creating the fbdev, gives the
possibility to specify the default fbdev size by firstly checking if the
command line is defining a preferred mode. Else go into fallback and set
1024x768 fbdev size as it was already done.

Cc: Tomi Valkeinen 
Signed-off-by: Vincent Abriou 
---
Patch v2:
 add a break in the connector for loop when a first cmdline mode is found

 drivers/gpu/drm/drm_fb_helper.c | 34 +-
 1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 0ab6aaa..b38285f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1526,6 +1526,7 @@ static int drm_fb_helper_single_fb_probe(struct 
drm_fb_helper *fb_helper,
}

crtc_count = 0;
+
for (i = 0; i < fb_helper->crtc_count; i++) {
struct drm_display_mode *desired_mode;
struct drm_mode_set *mode_set;
@@ -1570,11 +1571,34 @@ static int drm_fb_helper_single_fb_probe(struct 
drm_fb_helper *fb_helper,
}

if (crtc_count == 0 || sizes.fb_width == -1 || sizes.fb_height == -1) {
-   /* hmm everyone went away - assume VGA cable just fell out
-  and will come back later. */
-   DRM_INFO("Cannot find any crtc or sizes - going 1024x768\n");
-   sizes.fb_width = sizes.surface_width = 1024;
-   sizes.fb_height = sizes.surface_height = 768;
+   struct drm_display_mode *mode = NULL;
+   /* hmm everyone went away - assume cable just fell out and will
+* come back later.
+* Get fb size from command line mode (if existing) else fb size
+* is set to 1024x768
+*/
+   for (i = 0; i < fb_helper->connector_count; i++) {
+   struct drm_fb_helper_connector *fb_helper_conn;
+
+   fb_helper_conn = fb_helper->connector_info[i];
+   mode = drm_pick_cmdline_mode(fb_helper_conn);
+   if (mode)
+   break;
+   }
+
+   if (mode) {
+   sizes.fb_width = mode->hdisplay;
+   sizes.fb_height = mode->vdisplay;
+   DRM_INFO("Cannot find any crtc or sizes - use cmdline 
%dx%d\n",
+sizes.fb_width, sizes.fb_height);
+   } else {
+   sizes.fb_width = 1024;
+   sizes.fb_height = 768;
+   DRM_INFO("Cannot find any crtc or sizes - going 
1024x768\n");
+   }
+
+   sizes.surface_width = sizes.fb_width;
+   sizes.surface_height = sizes.fb_height;
}

/* push down into drivers */
-- 
2.7.4

[PATCH 2/2] nouveau: wrap the output poll execution and wakeup gpu

[Dave Airlie]

From: Dave Airlie 

This wraps the output poll and fixes a problem where
pm resume would try and take the mode config mutex
but the output poll thread would already have taken it.

I found this while looking for another bug, I've no idea
yet if this fixes that problem.

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/nouveau/nv50_display.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index cb85cb7..75502cd 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -4323,10 +4323,26 @@ nv50_disp_atomic_state_alloc(struct drm_device *dev)
return >state;
 }

+static bool
+nv50_disp_output_poll_execute(struct drm_device *dev)
+{
+   int ret;
+   bool repoll;
+   ret = pm_runtime_get_sync(dev->dev);
+   if (ret < 0 && ret != -EACCES)
+   return true;
+
+   repoll = drm_kms_helper_poll_outputs(dev);
+   pm_runtime_mark_last_busy(dev->dev);
+   pm_runtime_put_autosuspend(dev->dev);
+   return repoll;
+}
+
 static const struct drm_mode_config_funcs
 nv50_disp_func = {
.fb_create = nouveau_user_framebuffer_create,
.output_poll_changed = nouveau_fbcon_output_poll_changed,
+   .output_poll_execute = nv50_disp_output_poll_execute,
.atomic_check = nv50_disp_atomic_check,
.atomic_commit = nv50_disp_atomic_commit,
.atomic_state_alloc = nv50_disp_atomic_state_alloc,
-- 
2.9.3

[PATCH 1/2] drm: allow drivers to wrap output poll execution

[Dave Airlie]

From: Dave Airlie 

In order to avoid a lockdep between pm resume and output poll
over the mode_config mutex we need to order the wakeup of the
device before we take the mode config mutex (as pm resume
can also try and take the mode config mutex).

This introduces an API for drivers to insert themselves into
the output poll path to do pm gets before taking the mutex.

Signed-off-by: Dave Airlie 
---
 drivers/gpu/drm/drm_probe_helper.c | 25 ++---
 include/drm/drm_crtc_helper.h  |  2 +-
 include/drm/drm_mode_config.h  |  8 
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 97a3289..a2896d8 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -380,10 +380,15 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_hotplug_event);

-static void output_poll_execute(struct work_struct *work)
+/**
+ * drm_kms_helper_poll_outputs - poll all the outputs for hotplug
+ * @dev: drm_device to probe outputs on.
+ *
+ * This is to be called by drivers that need to wrap the output poll
+ * execution for power management purposes.
+ */
+bool drm_kms_helper_poll_outputs(struct drm_device *dev)
 {
-   struct delayed_work *delayed_work = to_delayed_work(work);
-   struct drm_device *dev = container_of(delayed_work, struct drm_device, 
mode_config.output_poll_work);
struct drm_connector *connector;
struct drm_connector_list_iter conn_iter;
enum drm_connector_status old_status;
@@ -463,6 +468,20 @@ static void output_poll_execute(struct work_struct *work)
if (changed)
drm_kms_helper_hotplug_event(dev);

+   return repoll;
+}
+EXPORT_SYMBOL(drm_kms_helper_poll_outputs);
+
+static void output_poll_execute(struct work_struct *work)
+{
+   struct delayed_work *delayed_work = to_delayed_work(work);
+   struct drm_device *dev = container_of(delayed_work, struct drm_device, 
mode_config.output_poll_work);
+   bool repoll;
+
+   if (dev->mode_config.funcs->output_poll_execute)
+   repoll = dev->mode_config.funcs->output_poll_execute(dev);
+   else
+   repoll = drm_kms_helper_poll_outputs(dev);
if (repoll)
schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
 }
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 982c299..861f9b0 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -74,5 +74,5 @@ extern void drm_kms_helper_hotplug_event(struct drm_device 
*dev);
 extern void drm_kms_helper_poll_disable(struct drm_device *dev);
 extern void drm_kms_helper_poll_enable(struct drm_device *dev);
 extern void drm_kms_helper_poll_enable_locked(struct drm_device *dev);
-
+extern bool drm_kms_helper_poll_outputs(struct drm_device *dev);
 #endif
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 17942c0..e9fc078 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -87,6 +87,14 @@ struct drm_mode_config_funcs {
void (*output_poll_changed)(struct drm_device *dev);

/**
+* @output_poll_execute:
+*
+* Used to allow driver to wrap the output polling for power management
+* purposes and avoid locking issues.
+*/
+   bool (*output_poll_execute)(struct drm_device *dev);
+
+   /**
 * @atomic_check:
 *
 * This is the only hook to validate an atomic modeset update. This
-- 
2.9.3

[rfc] fix output poll execute lockdep

[Dave Airlie]

On runtime resume, nouveau can try and take the mode_config
mutex in the poll reenable, however a poll can race with this,
and take the mutex and get stuck waiting for the runtime to
finish completion. These two patches allow the driver to
get hooked in before the mutex is taken.

I think radeon/amdgpu will also need similar patches to nouveau.

I found this while trying to track down a runtime regression
on W541 laptop, I'm not sure I found what the reporter was seeing yet.

Dave.

[PATCH v2] drm: Schedule the output_poll_work with 1s delay if we have delayed event

[Daniel Vetter]

On Tue, Jan 10, 2017 at 11:40:59AM +0100, Daniel Vetter wrote:
> On Mon, Jan 09, 2017 at 11:50:59AM -0500, Sean Paul wrote:
> > On Mon, Jan 9, 2017 at 9:31 AM, Peter Ujfalusi  
> > wrote:
> > > Instead of scheduling the work to handle the initial delayed event, use 1s
> > > delay.
> > >
> > > This delay should not be needed, but Optimus/nouveau will fail in a
> > > mysterious way if the delayed event is handled as soon as possible like it
> > 
> > Has anyone tried to demystify the failure? It seems like fixing the
> > root problem would be better than this.
> 
> Peter is on it, but fixing the regression meanwhile has priority imo.
> 
> > Perhaps we should just revert 339fd36238dd to fix stable.
> 
> That will make people unhappy about the delay again, so I think 1s delay
> is the better option.

For the future: If you add an in-patch changelog then it'd would have been
clear that we discussed all this already. Also good to cc all previous
commenters when submitting a new version.
-Daniel

> 
> > 
> > Sean
> > 
> > > is done in drm_helper_probe_single_connector_modes() in case the poll
> > > was enabled before.
> > >
> > > Reverting 339fd36238dd would give back the 10 sec (!) delay to handle the
> > > delayed event. Adding 1sec delay to the poll_work is enough to work around
> > > the issue in Optimus setups and gives shorter response on handling the
> > > initial delayed event.
> > >
> > > Fixes: 339fd36238dd ("drm: drm_probe_helper: Fix output_poll_work 
> > > scheduling")
> > > Cc: stable at vger.kernel.org   # v4.9
> > > Signed-off-by: Peter Ujfalusi 
> > > ---
> > >  drivers/gpu/drm/drm_probe_helper.c | 10 +-
> > >  1 file changed, 9 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> > > b/drivers/gpu/drm/drm_probe_helper.c
> > > index 06a62e37fbdc..258abed43e38 100644
> > > --- a/drivers/gpu/drm/drm_probe_helper.c
> > > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > > @@ -146,8 +146,16 @@ void drm_kms_helper_poll_enable_locked(struct 
> > > drm_device *dev)
> > > drm_connector_list_iter_put(_iter);
> > >
> > > if (dev->mode_config.delayed_event) {
> > > +   /*
> 
> I added a FIXME: heading here to make it stick out more, and then applied
> the patch.
> 
> Thanks, Daniel
> 
> > > +* Use short (1s) delay to handle the initial delayed 
> > > event.
> > > +* This delay should not be needed, but Optimus/nouveau 
> > > will
> > > +* fail in a mysterious way if the delayed event is 
> > > handled as
> > > +* soon as possible like it is done in
> > > +* drm_helper_probe_single_connector_modes() in case the 
> > > poll
> > > +* was enabled before.
> > > +*/
> > > poll = true;
> > > -   delay = 0;
> > > +   delay = HZ;
> > > }
> > >
> > > if (poll)
> > > --
> > > 2.11.0
> > >
> > 
> > 
> > 
> > -- 
> > Sean Paul, Software Engineer, Google / Chromium OS
> > ___
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH v2] drm: Schedule the output_poll_work with 1s delay if we have delayed event

[Daniel Vetter]

On Mon, Jan 09, 2017 at 11:50:59AM -0500, Sean Paul wrote:
> On Mon, Jan 9, 2017 at 9:31 AM, Peter Ujfalusi  
> wrote:
> > Instead of scheduling the work to handle the initial delayed event, use 1s
> > delay.
> >
> > This delay should not be needed, but Optimus/nouveau will fail in a
> > mysterious way if the delayed event is handled as soon as possible like it
> 
> Has anyone tried to demystify the failure? It seems like fixing the
> root problem would be better than this.

Peter is on it, but fixing the regression meanwhile has priority imo.

> Perhaps we should just revert 339fd36238dd to fix stable.

That will make people unhappy about the delay again, so I think 1s delay
is the better option.

> 
> Sean
> 
> > is done in drm_helper_probe_single_connector_modes() in case the poll
> > was enabled before.
> >
> > Reverting 339fd36238dd would give back the 10 sec (!) delay to handle the
> > delayed event. Adding 1sec delay to the poll_work is enough to work around
> > the issue in Optimus setups and gives shorter response on handling the
> > initial delayed event.
> >
> > Fixes: 339fd36238dd ("drm: drm_probe_helper: Fix output_poll_work 
> > scheduling")
> > Cc: stable at vger.kernel.org   # v4.9
> > Signed-off-by: Peter Ujfalusi 
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c | 10 +-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_probe_helper.c 
> > b/drivers/gpu/drm/drm_probe_helper.c
> > index 06a62e37fbdc..258abed43e38 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -146,8 +146,16 @@ void drm_kms_helper_poll_enable_locked(struct 
> > drm_device *dev)
> > drm_connector_list_iter_put(_iter);
> >
> > if (dev->mode_config.delayed_event) {
> > +   /*

I added a FIXME: heading here to make it stick out more, and then applied
the patch.

Thanks, Daniel

> > +* Use short (1s) delay to handle the initial delayed event.
> > +* This delay should not be needed, but Optimus/nouveau will
> > +* fail in a mysterious way if the delayed event is handled 
> > as
> > +* soon as possible like it is done in
> > +* drm_helper_probe_single_connector_modes() in case the 
> > poll
> > +* was enabled before.
> > +*/
> > poll = true;
> > -   delay = 0;
> > +   delay = HZ;
> > }
> >
> > if (poll)
> > --
> > 2.11.0
> >
> 
> 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> ___
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[PATCH 1/4] drm: add vblank hooks to struct drm_crtc_funcs

[Daniel Vetter]

On Mon, Jan 09, 2017 at 07:56:24PM +0800, Shawn Guo wrote:
> From: Shawn Guo 
> 
> The vblank is mostly CRTC specific and implemented as part of CRTC
> driver.  So having vblank hooks in struct drm_crtc_funcs should
> generally help to reduce code from client drivers in implementing
> drm_driver's vblank callbacks.
> 
> Signed-off-by: Shawn Guo 
> ---
>  drivers/gpu/drm/drm_crtc.c | 36 
>  include/drm/drm_crtc.h | 21 +
>  2 files changed, 57 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 85a7452d0fb4..59ff00f48101 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -70,6 +70,42 @@ struct drm_crtc *drm_crtc_from_index(struct drm_device 
> *dev, int idx)
>  EXPORT_SYMBOL(drm_crtc_from_index);
>  
>  /**
> + * drm_crtc_enable_vblank - vblank enable callback helper
> + * @dev: DRM device
> + * @pipe: CRTC index
> + *
> + * It's a helper function as the generic vblank enable callback 
> implementation,
> + * which calls into _crtc_funcs.enable_vblank function.
> + */
> +int drm_crtc_enable_vblank(struct drm_device *dev, unsigned int pipe)
> +{
> + struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> +
> + if (crtc && crtc->funcs && crtc->funcs->enable_vblank)
> + return crtc->funcs->enable_vblank(crtc);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_crtc_enable_vblank);

With the helper approach here there's still a pile of boilerplate in
drivers (well, 2 lines to fill out the legacy helpers). What if instead we
wrap all callers of enable/disable_vblank in drm_irq.c into something like

__enable_vblank(dev, pipe)
{
if (DRIVER_MODESET) /* otherwise we'll oops on legacy drivers */
{
/* above code to call the new hook, if it's there. */

if (crtc->funcs->enable_vblank)
return crtc->funcs->enable_vblank(crtc);
}

/* fallback for everyone else */

dev->driver->enable_vblank(dev, pipe);
}

> +
> +/**
> + * drm_crtc_disable_vblank - vblank disable callback helper
> + * @dev: DRM device
> + * @pipe: CRTC index
> + *
> + * It's a helper function as the generic vblank disable callback 
> implementation,
> + * which calls into _crtc_funcs.disable_vblank function.
> + */
> +void drm_crtc_disable_vblank(struct drm_device *dev, unsigned int pipe)
> +{
> + struct drm_crtc *crtc = drm_crtc_from_index(dev, pipe);
> +
> + if (crtc && crtc->funcs && crtc->funcs->disable_vblank)
> + return crtc->funcs->disable_vblank(crtc);
> +}
> +EXPORT_SYMBOL(drm_crtc_disable_vblank);
> +
> +/**
>   * drm_crtc_force_disable - Forcibly turn off a CRTC
>   * @crtc: CRTC to turn off
>   *
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a5627eb8621f..20874b3903a6 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -599,6 +599,25 @@ struct drm_crtc_funcs {
>*/
>   void (*atomic_print_state)(struct drm_printer *p,
>  const struct drm_crtc_state *state);
> +
> + /**
> +  * @enable_vblank:
> +  *
> +  * Enable vblank interrupts for the CRTC.
> +  *
> +  * Returns:
> +  *
> +  * Zero on success, appropriate errno if the vblank interrupt cannot
> +  * be enabled.
> +  */
> + int (*enable_vblank)(struct drm_crtc *crtc);
> +
> + /**
> +  * @disable_vblank:
> +  *
> +  * Disable vblank interrupts for the CRTC.
> +  */
> + void (*disable_vblank)(struct drm_crtc *crtc);

Please also update the kerneldoc for these two hooks in drm_drv.h, and
link between the two (using the _driver.enable_vblank syntax). And
then mention that the drm_driver one is deprecated.

Also I think it'd would make sense to do the same for
get_vblank_counter(), since those are the 3 core->driver interface
functions. The other 2 are kinda just helpers for precise vblank
timestamp support.
-Daniel

>  };
>  
>  /**
> @@ -831,6 +850,8 @@ void drm_crtc_get_hv_timing(const struct drm_display_mode 
> *mode,
>  
>  int drm_mode_set_config_internal(struct drm_mode_set *set);
>  struct drm_crtc *drm_crtc_from_index(struct drm_device *dev, int idx);
> +int drm_crtc_enable_vblank(struct drm_device *dev, unsigned int pipe);
> +void drm_crtc_disable_vblank(struct drm_device *dev, unsigned int pipe);
>  
>  /* Helpers */
>  static inline struct drm_crtc *drm_crtc_find(struct drm_device *dev,
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

[Intel-gfx] 4.10-rc2 oops in DRM connector code

[Daniel Vetter]

On Mon, Jan 09, 2017 at 09:42:22AM -0800, Dave Hansen wrote:
> On 01/09/2017 08:59 AM, Daniel Vetter wrote:
> > On Mon, Jan 9, 2017 at 5:50 PM, Dave Hansen  
> > wrote:
> >> On 01/09/2017 08:41 AM, Daniel Vetter wrote:
> >>> On Mon, Jan 9, 2017 at 2:40 PM, Dave Hansen  
> >>> wrote:
>  Well, now I found where the -2 comes from.
>  intel_dp_register_mst_connector() calls drm_connector_register(), which
>  fails to add the kobject (warning below).  But, it does zero error
>  checking on the drm_connector_register() call and leaves the
>  partially-constructed connector in place.
> 
>  The next time some poor, hapless code goes and tries to do anything with
>  that kdev, they oops.  I'm perplexed by this, though.  The
>  drm_dp_mst_topology_cbs->register_connector just returns void.  It seems
>  a bit goofy that it can't even _return_ failure.
> 
>  Is there some stable code to go back to here?  Or, is there something
>  about my configuration that's unique?  I really wonder why nobody else
>  is running into this.
> 
>  There's probably some other race going on here.  This warning doesn't
>  happen on every boot.
> >>> This smells more like the root-cause: Something goes wrong on boot
> >>> that prevents connectors from properly registering, then we fall over
> >>> later on. And the register callback is intentionally void, assuming
> >>> that any prep work has been done earlier and that therefore the
> >>> register step can't fail. Can you pls check whether the oops later on
> >>> only happens together with this warning at boot, or whether they're
> >>> not correlated?
> >>
> >> Looking through my logs, I can't find any instance of the oops without
> >> the warning at boot.  So I do think the later oops is entirely caused by
> >> the issue warned about in early boot.
> > 
> > Hm, I guess then we'd need to fix that boot-up warning. Can you try to
> > figure out why it's unhappy? On a hunch it could be that we call
> > drm_connector_register from the mst probe worker before the main
> > driver load thread has reached the drm_dev_register call. A few printk
> > to decide whether that's the case (plus a few boot-up tests to gather
> > the statistics, sorry about that) would be real great.
> 
> I think your suspicion is correct.
> 
> The WARNING is getting printed from CPU0 (the
> drm_dp_mst_link_probe_work() thread) and we can see the
> drm_connector_register()s happening in parallel on CPU1 while the
> WARNING is getting printed out.
> 
> This seems to happen very consistently when i915 is built into the kernel.

Do you have

commit e73ab00e9a0f1731f34d0620a9c55f5c30c4ad4e
Author: Daniel Vetter 
Date:   Sun Dec 18 14:35:45 2016 +0100

drm: prevent double-(un)registration for connectors

Lack of that would perfectly explain that oops ... Otherwise still no idea
what's going wrong.
-Daniel

> 
> > [1.404237] drm_dev_register(02da)::209092608 cpu: 1
> > [1.404284] drm_connector_register(88040ed3e800)::379 
> > connector->registered: 0 cpu: 0
> > [1.404293] sysfs_create_dir_ns()::53 error: -2
> > [1.404295] create_dir()::75 error: -2 cpu: 0
> > [1.404298] [ cut here ]
> > [1.404304] WARNING: CPU: 0 PID: 3 at lib/kobject.c:249 
> > kobject_add_internal+0x273/0x330
> > [1.404308] kobject_add_internal failed for card0-DP-3 (error: -2 
> > parent: card0)
> > [1.404311] Modules linked in:
> > [1.404316] CPU: 0 PID: 3 Comm: kworker/0:0 Not tainted 
> > 4.10.0-rc3-i915borked-dirty #66
> > [1.404319] Hardware name: LENOVO 20F5S7V800/20F5S7V800, BIOS R02ET50W 
> > (1.23 ) 09/20/2016
> > [1.404325] Workqueue: events_long drm_dp_mst_link_probe_work
> > [1.404328] Call Trace:
> > [1.404334]  dump_stack+0x67/0x99
> > [1.404338]  __warn+0xd1/0xf0
> > [1.404342]  warn_slowpath_fmt+0x4f/0x60
> > [1.404345]  kobject_add_internal+0x273/0x330
> > [1.404347]  kobject_add+0x65/0xb0
> > [1.404352]  ? klist_init+0x31/0x40
> > [1.404356]  device_add+0x102/0x5d0
> > [1.404360]  ? kfree_const+0x22/0x30
> > [1.404364]  device_create_groups_vargs+0xd8/0x100
> > [1.404367]  device_create_with_groups+0x36/0x40
> > [1.404370] drm_connector_register(88040c779000)::379 
> > connector->registered: 0 cpu: 1
> > [1.404374]  ? vprintk_default+0x29/0x50
> > [1.404375]  ? printk+0x4d/0x4f
> > [1.404378]  drm_sysfs_connector_add+0x60/0xe0
> > [1.404381]  drm_connector_register+0x4e/0x100
> > [1.404383]  intel_dp_register_mst_connector+0x41/0x50
> > [1.404385]  drm_dp_add_port+0x350/0x450
> > [1.404388]  ? rcu_early_boot_tests+0x1/0x10
> > [1.404390]  ? schedule_timeout+0x1cd/0x390
> > [1.404392]  ? __might_sleep+0x4a/0x90
> > [1.404394]  ? mutex_lock+0x25/0x50
> > [1.404395]  ? drm_dp_mst_wait_tx_reply+0x118/0x1e0
> > [1.404397]  ? prepare_to_wait_event+0x120/0x120
> > [1.404399]  ? 

[PATCH v2 7/7] drm/blend: Use new atomic iterator macros.

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_blend.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_blend.c b/drivers/gpu/drm/drm_blend.c
index 5c45d0852608..78cf9f6cae08 100644
--- a/drivers/gpu/drm/drm_blend.c
+++ b/drivers/gpu/drm/drm_blend.c
@@ -378,26 +378,26 @@ int drm_atomic_normalize_zpos(struct drm_device *dev,
  struct drm_atomic_state *state)
 {
struct drm_crtc *crtc;
-   struct drm_crtc_state *crtc_state;
+   struct drm_crtc_state *old_crtc_state, *new_crtc_state;
struct drm_plane *plane;
-   struct drm_plane_state *plane_state;
+   struct drm_plane_state *old_plane_state, *new_plane_state;
int i, ret = 0;

-   for_each_plane_in_state(state, plane, plane_state, i) {
-   crtc = plane_state->crtc;
+   for_each_oldnew_plane_in_state(state, plane, old_plane_state, 
new_plane_state, i) {
+   crtc = new_plane_state->crtc;
if (!crtc)
continue;
-   if (plane->state->zpos != plane_state->zpos) {
-   crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
-   crtc_state->zpos_changed = true;
+   if (old_plane_state->zpos != new_plane_state->zpos) {
+   new_crtc_state = drm_atomic_get_new_crtc_state(state, 
crtc);
+   new_crtc_state->zpos_changed = true;
}
}

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
-   if (crtc_state->plane_mask != crtc->state->plane_mask ||
-   crtc_state->zpos_changed) {
+   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
+   if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
+   new_crtc_state->zpos_changed) {
ret = drm_atomic_helper_crtc_normalize_zpos(crtc,
-   crtc_state);
+   
new_crtc_state);
if (ret)
return ret;
}
-- 
2.7.4

[PATCH v2 6/7] drm/atomic: Convert get_existing_state callers to get_old/new_state

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic.c|  6 +++---
 drivers/gpu/drm/drm_atomic_helper.c | 39 +
 drivers/gpu/drm/drm_blend.c |  3 +--
 3 files changed, 22 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7b61e0da9ace..6428e9469607 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1362,8 +1362,8 @@ drm_atomic_set_crtc_for_connector(struct 
drm_connector_state *conn_state,
return 0;

if (conn_state->crtc) {
-   crtc_state = 
drm_atomic_get_existing_crtc_state(conn_state->state,
-   
conn_state->crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(conn_state->state,
+  conn_state->crtc);

crtc_state->connector_mask &=
~(1 << drm_connector_index(conn_state->connector));
@@ -1480,7 +1480,7 @@ drm_atomic_add_affected_planes(struct drm_atomic_state 
*state,
 {
struct drm_plane *plane;

-   WARN_ON(!drm_atomic_get_existing_crtc_state(state, crtc));
+   WARN_ON(!drm_atomic_get_new_crtc_state(state, crtc));

drm_for_each_plane_mask(plane, state->dev, crtc->state->plane_mask) {
struct drm_plane_state *plane_state =
diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index b577977322d0..0b770eee0958 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -70,8 +70,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
struct drm_crtc_state *crtc_state;

if (old_plane_state->crtc) {
-   crtc_state = drm_atomic_get_existing_crtc_state(state,
-   
old_plane_state->crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, 
old_plane_state->crtc);

if (WARN_ON(!crtc_state))
return;
@@ -80,8 +79,7 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state 
*state,
}

if (plane_state->crtc) {
-   crtc_state = drm_atomic_get_existing_crtc_state(state,
-   
plane_state->crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, 
plane_state->crtc);

if (WARN_ON(!crtc_state))
return;
@@ -150,7 +148,7 @@ static int handle_conflicting_encoders(struct 
drm_atomic_state *state,
drm_for_each_connector_iter(connector, _iter) {
struct drm_crtc_state *crtc_state;

-   if (drm_atomic_get_existing_connector_state(state, connector))
+   if (drm_atomic_get_new_connector_state(state, connector))
continue;

encoder = connector->state->best_encoder;
@@ -178,7 +176,7 @@ static int handle_conflicting_encoders(struct 
drm_atomic_state *state,
 conn_state->crtc->base.id, 
conn_state->crtc->name,
 connector->base.id, connector->name);

-   crtc_state = drm_atomic_get_existing_crtc_state(state, 
conn_state->crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, 
conn_state->crtc);

ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
if (ret)
@@ -219,7 +217,7 @@ set_best_encoder(struct drm_atomic_state *state,
 */
WARN_ON(!crtc && encoder != conn_state->best_encoder);
if (crtc) {
-   crtc_state = drm_atomic_get_existing_crtc_state(state, 
crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, crtc);

crtc_state->encoder_mask &=
~(1 << 
drm_encoder_index(conn_state->best_encoder));
@@ -230,7 +228,7 @@ set_best_encoder(struct drm_atomic_state *state,
crtc = conn_state->crtc;
WARN_ON(!crtc);
if (crtc) {
-   crtc_state = drm_atomic_get_existing_crtc_state(state, 
crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, crtc);

crtc_state->encoder_mask |=
1 << drm_encoder_index(encoder);
@@ -263,7 +261,7 @@ steal_encoder(struct drm_atomic_state *state,

set_best_encoder(state, new_connector_state, NULL);

-   crtc_state = drm_atomic_get_existing_crtc_state(state, 
encoder_crtc);
+   crtc_state = drm_atomic_get_new_crtc_state(state, encoder_crtc);
crtc_state->connectors_changed = true;

return;
@@ -286,12 +284,12 @@ update_connector_routing(struct drm_atomic_state *state,

if 

[PATCH v2 5/7] drm/atomic: Add macros to access existing old/new state

[Maarten Lankhorst]

After atomic commit, these macros should be used in place of
get_existing_state. Also after commit get_xx_state should no longer
be used because it may not have the required locks.

Signed-off-by: Maarten Lankhorst 
---
 include/drm/drm_atomic.h | 99 
 1 file changed, 99 insertions(+)

diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
index 6062e7f27325..2e6bb7acc837 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -285,6 +285,35 @@ drm_atomic_get_existing_crtc_state(struct drm_atomic_state 
*state,
 }

 /**
+ * drm_atomic_get_old_crtc_state - get old crtc state, if it exists
+ * @state: global atomic state object
+ * @crtc: crtc to grab
+ *
+ * This function returns the old crtc state for the given crtc, or
+ * NULL if the crtc is not part of the global atomic state.
+ */
+static inline struct drm_crtc_state *
+drm_atomic_get_old_crtc_state(struct drm_atomic_state *state,
+ struct drm_crtc *crtc)
+{
+   return state->crtcs[drm_crtc_index(crtc)].old_state;
+}
+/**
+ * drm_atomic_get_new_crtc_state - get new crtc state, if it exists
+ * @state: global atomic state object
+ * @crtc: crtc to grab
+ *
+ * This function returns the new crtc state for the given crtc, or
+ * NULL if the crtc is not part of the global atomic state.
+ */
+static inline struct drm_crtc_state *
+drm_atomic_get_new_crtc_state(struct drm_atomic_state *state,
+ struct drm_crtc *crtc)
+{
+   return state->crtcs[drm_crtc_index(crtc)].new_state;
+}
+
+/**
  * drm_atomic_get_existing_plane_state - get plane state, if it exists
  * @state: global atomic state object
  * @plane: plane to grab
@@ -300,6 +329,36 @@ drm_atomic_get_existing_plane_state(struct 
drm_atomic_state *state,
 }

 /**
+ * drm_atomic_get_old_plane_state - get plane state, if it exists
+ * @state: global atomic state object
+ * @plane: plane to grab
+ *
+ * This function returns the old plane state for the given plane, or
+ * NULL if the plane is not part of the global atomic state.
+ */
+static inline struct drm_plane_state *
+drm_atomic_get_old_plane_state(struct drm_atomic_state *state,
+  struct drm_plane *plane)
+{
+   return state->planes[drm_plane_index(plane)].old_state;
+}
+
+/**
+ * drm_atomic_get_new_plane_state - get plane state, if it exists
+ * @state: global atomic state object
+ * @plane: plane to grab
+ *
+ * This function returns the new plane state for the given plane, or
+ * NULL if the plane is not part of the global atomic state.
+ */
+static inline struct drm_plane_state *
+drm_atomic_get_new_plane_state(struct drm_atomic_state *state,
+  struct drm_plane *plane)
+{
+   return state->planes[drm_plane_index(plane)].new_state;
+}
+
+/**
  * drm_atomic_get_existing_connector_state - get connector state, if it exists
  * @state: global atomic state object
  * @connector: connector to grab
@@ -320,6 +379,46 @@ drm_atomic_get_existing_connector_state(struct 
drm_atomic_state *state,
 }

 /**
+ * drm_atomic_get_old_connector_state - get connector state, if it exists
+ * @state: global atomic state object
+ * @connector: connector to grab
+ *
+ * This function returns the old connector state for the given connector,
+ * or NULL if the connector is not part of the global atomic state.
+ */
+static inline struct drm_connector_state *
+drm_atomic_get_old_connector_state(struct drm_atomic_state *state,
+  struct drm_connector *connector)
+{
+   int index = drm_connector_index(connector);
+
+   if (index >= state->num_connector)
+   return NULL;
+
+   return state->connectors[index].old_state;
+}
+
+/**
+ * drm_atomic_get_new_connector_state - get connector state, if it exists
+ * @state: global atomic state object
+ * @connector: connector to grab
+ *
+ * This function returns the new connector state for the given connector,
+ * or NULL if the connector is not part of the global atomic state.
+ */
+static inline struct drm_connector_state *
+drm_atomic_get_new_connector_state(struct drm_atomic_state *state,
+  struct drm_connector *connector)
+{
+   int index = drm_connector_index(connector);
+
+   if (index >= state->num_connector)
+   return NULL;
+
+   return state->connectors[index].new_state;
+}
+
+/**
  * __drm_atomic_get_current_plane_state - get current plane state
  * @state: global atomic state object
  * @plane: plane to grab
-- 
2.7.4

[PATCH v2 4/7] drm/atomic: Fix atomic helpers to use the new iterator macros.

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic_helper.c | 293 +++-
 1 file changed, 152 insertions(+), 141 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
b/drivers/gpu/drm/drm_atomic_helper.c
index c468194934a8..b577977322d0 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -63,14 +63,15 @@
  */
 static void
 drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
+   struct drm_plane_state *old_plane_state,
struct drm_plane_state *plane_state,
struct drm_plane *plane)
 {
struct drm_crtc_state *crtc_state;

-   if (plane->state->crtc) {
+   if (old_plane_state->crtc) {
crtc_state = drm_atomic_get_existing_crtc_state(state,
-   
plane->state->crtc);
+   
old_plane_state->crtc);

if (WARN_ON(!crtc_state))
return;
@@ -104,7 +105,7 @@ static int handle_conflicting_encoders(struct 
drm_atomic_state *state,
 * part of the state. If the same encoder is assigned to multiple
 * connectors bail out.
 */
-   for_each_connector_in_state(state, connector, conn_state, i) {
+   for_each_new_connector_in_state(state, connector, conn_state, i) {
const struct drm_connector_helper_funcs *funcs = 
connector->helper_private;
struct drm_encoder *new_encoder;

@@ -245,22 +246,22 @@ steal_encoder(struct drm_atomic_state *state,
 {
struct drm_crtc_state *crtc_state;
struct drm_connector *connector;
-   struct drm_connector_state *connector_state;
+   struct drm_connector_state *old_connector_state, *new_connector_state;
int i;

-   for_each_connector_in_state(state, connector, connector_state, i) {
+   for_each_oldnew_connector_in_state(state, connector, 
old_connector_state, new_connector_state, i) {
struct drm_crtc *encoder_crtc;

-   if (connector_state->best_encoder != encoder)
+   if (new_connector_state->best_encoder != encoder)
continue;

-   encoder_crtc = connector->state->crtc;
+   encoder_crtc = old_connector_state->crtc;

DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s], 
stealing it\n",
 encoder->base.id, encoder->name,
 encoder_crtc->base.id, encoder_crtc->name);

-   set_best_encoder(state, connector_state, NULL);
+   set_best_encoder(state, new_connector_state, NULL);

crtc_state = drm_atomic_get_existing_crtc_state(state, 
encoder_crtc);
crtc_state->connectors_changed = true;
@@ -272,7 +273,8 @@ steal_encoder(struct drm_atomic_state *state,
 static int
 update_connector_routing(struct drm_atomic_state *state,
 struct drm_connector *connector,
-struct drm_connector_state *connector_state)
+struct drm_connector_state *old_connector_state,
+struct drm_connector_state *new_connector_state)
 {
const struct drm_connector_helper_funcs *funcs;
struct drm_encoder *new_encoder;
@@ -282,24 +284,24 @@ update_connector_routing(struct drm_atomic_state *state,
 connector->base.id,
 connector->name);

-   if (connector->state->crtc != connector_state->crtc) {
-   if (connector->state->crtc) {
-   crtc_state = drm_atomic_get_existing_crtc_state(state, 
connector->state->crtc);
+   if (old_connector_state->crtc != new_connector_state->crtc) {
+   if (old_connector_state->crtc) {
+   crtc_state = drm_atomic_get_existing_crtc_state(state, 
old_connector_state->crtc);
crtc_state->connectors_changed = true;
}

-   if (connector_state->crtc) {
-   crtc_state = drm_atomic_get_existing_crtc_state(state, 
connector_state->crtc);
+   if (new_connector_state->crtc) {
+   crtc_state = drm_atomic_get_existing_crtc_state(state, 
new_connector_state->crtc);
crtc_state->connectors_changed = true;
}
}

-   if (!connector_state->crtc) {
+   if (!new_connector_state->crtc) {
DRM_DEBUG_ATOMIC("Disabling [CONNECTOR:%d:%s]\n",
connector->base.id,
connector->name);

-   set_best_encoder(state, connector_state, NULL);
+   set_best_encoder(state, new_connector_state, NULL);

return 0;
}
@@ -308,7 +310,7 @@ 

[PATCH v2 3/7] drm/atomic: Use new atomic iterator macros.

[Maarten Lankhorst]

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 18cdf2c956c6..7b61e0da9ace 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1562,7 +1562,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)

DRM_DEBUG_ATOMIC("checking %p\n", state);

-   for_each_plane_in_state(state, plane, plane_state, i) {
+   for_each_new_plane_in_state(state, plane, plane_state, i) {
ret = drm_atomic_plane_check(plane, plane_state);
if (ret) {
DRM_DEBUG_ATOMIC("[PLANE:%d:%s] atomic core check 
failed\n",
@@ -1571,7 +1571,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
}
}

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
ret = drm_atomic_crtc_check(crtc, crtc_state);
if (ret) {
DRM_DEBUG_ATOMIC("[CRTC:%d:%s] atomic core check 
failed\n",
@@ -1584,7 +1584,7 @@ int drm_atomic_check_only(struct drm_atomic_state *state)
ret = config->funcs->atomic_check(state->dev, state);

if (!state->allow_modeset) {
-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
if (drm_atomic_crtc_needs_modeset(crtc_state)) {
DRM_DEBUG_ATOMIC("[CRTC:%d:%s] requires full 
modeset\n",
 crtc->base.id, crtc->name);
@@ -1668,13 +1668,13 @@ static void drm_atomic_print_state(const struct 
drm_atomic_state *state)

DRM_DEBUG_ATOMIC("checking %p\n", state);

-   for_each_plane_in_state(state, plane, plane_state, i)
+   for_each_new_plane_in_state(state, plane, plane_state, i)
drm_atomic_plane_print_state(, plane_state);

-   for_each_crtc_in_state(state, crtc, crtc_state, i)
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i)
drm_atomic_crtc_print_state(, crtc_state);

-   for_each_connector_in_state(state, connector, connector_state, i)
+   for_each_new_connector_in_state(state, connector, connector_state, i)
drm_atomic_connector_print_state(, connector_state);
 }

@@ -1961,7 +1961,7 @@ static int prepare_crtc_signaling(struct drm_device *dev,
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
return 0;

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
u64 __user *fence_ptr;

fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
@@ -2041,7 +2041,7 @@ static void complete_crtc_signaling(struct drm_device 
*dev,
return;
}

-   for_each_crtc_in_state(state, crtc, crtc_state, i) {
+   for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
/*
 * TEST_ONLY and PAGE_FLIP_EVENT are mutually
 * exclusive, if they weren't, this code should be
-- 
2.7.4

[PATCH v2 2/7] drm/atomic: Make add_affected_connectors look at crtc_state.

[Maarten Lankhorst]

This kills another dereference of connector->state. connector_mask
holds all unchanged connectors at least and any changed connectors
are already in state anyway.

Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 1c1cbf436717..18cdf2c956c6 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1419,8 +1419,13 @@ drm_atomic_add_affected_connectors(struct 
drm_atomic_state *state,
struct drm_connector *connector;
struct drm_connector_state *conn_state;
struct drm_connector_list_iter conn_iter;
+   struct drm_crtc_state *crtc_state;
int ret;

+   crtc_state = drm_atomic_get_crtc_state(state, crtc);
+   if (IS_ERR(crtc_state))
+   return PTR_ERR(crtc_state);
+
ret = drm_modeset_lock(>connection_mutex, state->acquire_ctx);
if (ret)
return ret;
@@ -1429,12 +1434,12 @@ drm_atomic_add_affected_connectors(struct 
drm_atomic_state *state,
 crtc->base.id, crtc->name, state);

/*
-* Changed connectors are already in @state, so only need to look at the
-* current configuration.
+* Changed connectors are already in @state, so only need to look
+* at the connector_mask in crtc_state.
 */
drm_connector_list_iter_get(state->dev, _iter);
drm_for_each_connector_iter(connector, _iter) {
-   if (connector->state->crtc != crtc)
+   if (!(crtc_state->connector_mask & (1 << 
drm_connector_index(connector
continue;

conn_state = drm_atomic_get_connector_state(state, connector);
-- 
2.7.4