Re: [Intel-gfx] [PATCH v5 2/3] drm: Add Gamma and Degamma LUT sizes props to drm_crtc to validate.

2021-11-04 Thread Ilia Mirkin
On Thu, Nov 4, 2021 at 4:03 PM Mark Yacoub  wrote:
>
> From: Mark Yacoub 
>
> [Why]
> 1. drm_atomic_helper_check doesn't check for the LUT sizes of either Gamma
> or Degamma props in the new CRTC state, allowing any invalid size to
> be passed on.
> 2. Each driver has its own LUT size, which could also be different for
> legacy users.
>
> [How]
> 1. Create |degamma_lut_size| and |gamma_lut_size| to save the LUT sizes
> assigned by the driver when it's initializing its color and CTM
> management.
> 2. Create drm_atomic_helper_check_crtc which is called by
> drm_atomic_helper_check to check the LUT sizes saved in drm_crtc that
> they match the sizes in the new CRTC state.
> 3. As the LUT size check now happens in drm_atomic_helper_check, remove
> the lut check in intel_color.c
>
> Resolves: igt@kms_color@pipe-A-invalid-gamma-lut-sizes on MTK
> Tested on Zork (amdgpu) and Jacuzzi (mediatek), volteer (TGL)
>
> v3:
> 1. Check for lut pointer inside drm_check_lut_size.
>
> v2:
> 1. Remove the rename to a parent commit.
> 2. Create a drm drm_check_lut_size instead of intel only function.
>
> v1:
> 1. Fix typos
> 2. Remove the LUT size check from intel driver
> 3. Rename old LUT check to indicate it's a channel change
>
> Signed-off-by: Mark Yacoub 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c| 52 ++
>  drivers/gpu/drm/drm_color_mgmt.c   | 19 
>  drivers/gpu/drm/i915/display/intel_color.c | 32 ++---
>  include/drm/drm_atomic_helper.h|  1 +
>  include/drm/drm_color_mgmt.h   |  3 ++
>  include/drm/drm_crtc.h | 11 +
>  6 files changed, 99 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index bc3487964fb5e..548e5d8221fb4 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -929,6 +929,54 @@ drm_atomic_helper_check_planes(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL(drm_atomic_helper_check_planes);
>
> +/**
> + * drm_atomic_helper_check_crtcs - validate state object for CRTC changes
> + * @state: the driver state object
> + *
> + * Check the CRTC state object such as the Gamma/Degamma LUT sizes if the new
> + * state holds them.
> + *
> + * RETURNS:
> + * Zero for success or -errno
> + */
> +int drm_atomic_helper_check_crtcs(struct drm_atomic_state *state)
> +{
> +   struct drm_crtc *crtc;
> +   struct drm_crtc_state *new_crtc_state;
> +   int i;
> +
> +   for_each_new_crtc_in_state (state, crtc, new_crtc_state, i) {
> +   if (!new_crtc_state->color_mgmt_changed)
> +   continue;
> +
> +   if (drm_check_lut_size(new_crtc_state->gamma_lut,
> +  crtc->gamma_lut_size) ||
> +   drm_check_lut_size(new_crtc_state->gamma_lut,
> +  crtc->gamma_size)) {
> +   drm_dbg_state(
> +   state->dev,
> +   "Invalid Gamma LUT size. Expected %u/%u, got 
> %u.\n",
> +   crtc->gamma_lut_size, crtc->gamma_size,
> +   
> drm_color_lut_size(new_crtc_state->gamma_lut));
> +   return -EINVAL;
> +   }
> +
> +   if (drm_check_lut_size(new_crtc_state->degamma_lut,
> +  crtc->degamma_lut_size)) {
> +   drm_dbg_state(
> +   state->dev,
> +   "Invalid Degamma LUT size. Expected %u, got 
> %u.\n",
> +   crtc->degamma_lut_size,
> +   drm_color_lut_size(
> +   new_crtc_state->degamma_lut));
> +   return -EINVAL;
> +   }
> +   }
> +
> +   return 0;
> +}
> +EXPORT_SYMBOL(drm_atomic_helper_check_crtcs);
> +
>  /**
>   * drm_atomic_helper_check - validate state object
>   * @dev: DRM device
> @@ -974,6 +1022,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
> if (ret)
> return ret;
>
> +   ret = drm_atomic_helper_check_crtcs(state);
> +   if (ret)
> +   return ret;
> +
> if (state->legacy_cursor_update)
> state->async_update = !drm_atomic_helper_async_check(dev, 
> state);
>
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c 
> b/drivers/gpu/drm/drm_color_mgmt.c
> index 16a07f84948f3..c85094223b535 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -166,6 +166,7 @@ void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> struct drm_mode_config *config = &dev->mode_config;
>
> if (degamma_lut_size) {
> +   crtc->degamma_lut_size = degamma_lut_size;
> drm_object_attach_property(&crtc->base,
> 

Re: [Intel-gfx] [Nouveau] [PATCH v4 02/17] drm/nouveau/kms/nv50-: Move AUX adapter reg to connector late register/early unregister

2021-04-23 Thread Ilia Mirkin
Some trivia, no comment on the real logic of the changes:

On Fri, Apr 23, 2021 at 2:43 PM Lyude Paul  wrote:
>
> Since AUX adapters on nouveau have their respective DRM connectors as
> parents, we need to make sure that we register then after their connectors.

then -> them

>
> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 25 -
>  1 file changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 61e6d7412505..c04044be3d32 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -401,7 +401,6 @@ nouveau_connector_destroy(struct drm_connector *connector)
> drm_connector_cleanup(connector);
> if (nv_connector->aux.transfer) {
> drm_dp_cec_unregister_connector(&nv_connector->aux);
> -   drm_dp_aux_unregister(&nv_connector->aux);
> kfree(nv_connector->aux.name);
> }
> kfree(connector);
> @@ -905,13 +904,29 @@ nouveau_connector_late_register(struct drm_connector 
> *connector)
> int ret;
>
> ret = nouveau_backlight_init(connector);
> +   if (ret)
> +   return ret;
>
> +   if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +   connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort) {
> +   ret = drm_dp_aux_register(&nouveau_connector(connector)->aux);
> +   if (ret)
> +   goto backlight_fini;
> +   }
> +
> +   return 0;
> +backlight_fini:
> +   nouveau_backlight_fini(connector);
> return ret;
>  }
>
>  static void
>  nouveau_connector_early_unregister(struct drm_connector *connector)
>  {
> +   if (connector->connector_type == DRM_MODE_CONNECTOR_eDP ||
> +   connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort)
> +   drm_dp_aux_unregister(&nouveau_connector(connector)->aux);
> +
> nouveau_backlight_fini(connector);
>  }
>
> @@ -1343,14 +1358,14 @@ nouveau_connector_create(struct drm_device *dev,
> snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
>  dcbe->hasht, dcbe->hashm);
> nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
> -   ret = drm_dp_aux_register(&nv_connector->aux);
> +   drm_dp_aux_init(&nv_connector->aux);
> if (ret) {
> -   NV_ERROR(drm, "failed to register aux channel\n");
> +   NV_ERROR(drm, "Failed to init AUX adapter for 
> sor-%04x-%04x: %d\n",

Maybe just use aux_name instead of rebuilding the string again?

> +dcbe->hasht, dcbe->hashm, ret);
> kfree(nv_connector);
> return ERR_PTR(ret);
> }
> -   funcs = &nouveau_connector_funcs;
> -   break;
> +   fallthrough;
> default:
> funcs = &nouveau_connector_funcs;
> break;
> --
> 2.30.2
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 18/18] drm/i915/display13: Enabling dithering after the CC1 pipe

2021-02-28 Thread Ilia Mirkin
Just wanted to mention ... nouveau supports two separate properties,
one controlling the type of dithering, and the other the dithering
depth:

dithering depth: auto
supported: auto, 6 bpc, 8 bpc
dithering mode: auto
supported: auto, off, static 2x2, dynamic 2x2, temporal

I think these are the properties Mario was alluding to. If I have an
8bpc buffer and set the dithering depth to 6bpc, it will dither down
to this. This is useful for LVDS panels primarily. Sometimes we know
it's 6bpc (and "auto" works), sometimes we don't.

These properties are largely a reflection of how the hardware works.
For example,

https://nvidia.github.io/open-gpu-doc/classes/display/cl917d.h
search for SET_DITHER_CONTROL.

Perhaps this "API" would not be appropriate for Intel HW, not sure.
But there's definitely precedent.

Cheers,

  -ilia

On Sun, Feb 28, 2021 at 11:57 PM Varide, Nischal
 wrote:
>
> Looks like there are two options.
>
> Enable or Disable Dithering via kernel command line or sysfs.
> To implement new Uapi.
>
> May be the first one is more feasible and faster
>
>
>
> Regards
>
> Nischal
>
>
>
> From: Mario Kleiner 
> Sent: Friday, February 19, 2021 11:15 AM
> To: Ville Syrjälä 
> Cc: Roper, Matthew D ; intel-gfx 
> ; Varide, Nischal 
> ; dri-devel 
> Subject: Re: [Intel-gfx] [PATCH 18/18] drm/i915/display13: Enabling dithering 
> after the CC1 pipe
>
>
>
>
>
>
>
> On Fri, Feb 19, 2021 at 4:22 AM Mario Kleiner  
> wrote:
>
>
>
>
>
> On Thu, Feb 11, 2021 at 1:29 PM Ville Syrjälä  
> wrote:
>
> On Thu, Jan 28, 2021 at 11:24:13AM -0800, Matt Roper wrote:
> > From: Nischal Varide 
> >
> > If the panel is 12bpc then Dithering is not enabled in the Legacy
> > dithering block , instead its Enabled after the C1 CC1 pipe post
> > color space conversion.For a 6bpc pannel Dithering is enabled in
> > Legacy block.
>
> Dithering is probably going to require a whole uapi bikeshed.
> Not sure we can just enable it unilaterally.
>
> Ccing dri-devel, and Mario who had issues with dithering in the
> past...
>
> Thanks for the cc Ville!
>
>
>
> The problem with dithering on Intel is that various tested Intel gpu's 
> (Ironlake, IvyBridge, Haswell, Skylake iirc.) are dithering when they 
> shouldn't. If one has a standard 8 bpc framebuffer feeding into a standard 
> (legacy) 256 slots, 8 bit wide lut which was loaded with an identity mapping, 
> feeding into a standard 8 bpc video output (DVI/HDMI/DP), the expected result 
> is that pixels rendered into the framebuffer show up unmodified at the video 
> output. What happens instead is that some dithering is needlessly applied. 
> This is bad for various neuroscience/medical research equipment that requires 
> pixels to pass unmodified in a pure 8 bpc configuration, e.g., because some 
> digital info is color-encoded in-band in the rendered image to control 
> research hardware, a la "if rgb pixel (123, 12, 23) is detected in the 
> digital video stream, emit some trigger signal, or timestamp that moment with 
> a hw clock, or start or stop some scientific recording equipment". Also there 
> exist specialized visual stimulators to drive special displays with more than 
> 12 bpc, e.g., 16 bpc, and so they encode the 8MSB of 16 bpc color values in 
> pixels in even columns, and the 8LSB in the odd columns of the framebuffer. 
> Unexpected dithering makes such equipment completely unusable. By now I must 
> have spent months of my life, just trying to deal with dithering induced 
> problems on different gpu's due to hw quirks or bugs somewhere in the 
> graphics stack.
>
>
>
> Atm. the intel kms driver disables dithering for anything with >= 8 bpc as a 
> fix for this harmful hardware quirk.
>
>
>
> Ideally we'd have uapi that makes dithering controllable per connector 
> (on/off/auto, selectable depth), also in a way that those controls are 
> exposed as RandR output properties, easily controllable by X clients. And 
> some safe default in case the client can't access the properties (like I'd 
> expect to happen with the dozens of Wayland compositors under the sun). 
> Various drivers had this over time, e.g., AMD classic kms path (if i don't 
> misremember) and nouveau, but some of it also got lost in the new atomic kms 
> variants, and Intel never exposed this.
>
>
>
> Or maybe some method that checks the values actually stored in the hw lut's, 
> CTM etc. and if the values suggest no dithering should be needed, disable the 
> dithering. E.g., if output depth is 8 bpc, one only needs dithering if the 
> slots in the final active hw lut do have any meaningful values in the lower 
> bits below the top 8 MSB, ie. if the content is actually > 8 bpc net bit 
> depth.
>
>
>
> -mario
>
>
>
>
>
> One cup of coffee later... I think this specific patch should be ok wrt. my 
> use cases. The majority of the above mentioned research devices are 
> single/dual-link DVI receivers, ie. 8 bpc video sinks. I'm only aware of one 
> recent device that has

Re: [Intel-gfx] [Nouveau] [RFC v3 05/10] drm/i915/dpcd_bl: Cleanup intel_dp_aux_vesa_enable_backlight() a bit

2021-02-05 Thread Ilia Mirkin
On Fri, Feb 5, 2021 at 6:45 PM Lyude Paul  wrote:
>
> Get rid of the extraneous switch case in here, and just open code
> edp_backlight_mode as we only ever use it once.
>
> Signed-off-by: Lyude Paul 
> ---
>  .../gpu/drm/i915/display/intel_dp_aux_backlight.c | 15 ++-
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c 
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index c37ccc8538cb..95e3e344cf40 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -382,7 +382,7 @@ intel_dp_aux_vesa_enable_backlight(const struct 
> intel_crtc_state *crtc_state,
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> struct intel_panel *panel = &connector->panel;
> -   u8 dpcd_buf, new_dpcd_buf, edp_backlight_mode;
> +   u8 dpcd_buf, new_dpcd_buf;
> u8 pwmgen_bit_count = panel->backlight.edp.vesa.pwmgen_bit_count;
>
> if (drm_dp_dpcd_readb(&intel_dp->aux,
> @@ -393,12 +393,8 @@ intel_dp_aux_vesa_enable_backlight(const struct 
> intel_crtc_state *crtc_state,
> }
>
> new_dpcd_buf = dpcd_buf;
> -   edp_backlight_mode = dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
>
> -   switch (edp_backlight_mode) {
> -   case DP_EDP_BACKLIGHT_CONTROL_MODE_PWM:
> -   case DP_EDP_BACKLIGHT_CONTROL_MODE_PRESET:
> -   case DP_EDP_BACKLIGHT_CONTROL_MODE_PRODUCT:
> +   if ((dpcd_buf & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) != 
> DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) {

You probably meant != MODE_DPCD?

> new_dpcd_buf &= ~DP_EDP_BACKLIGHT_CONTROL_MODE_MASK;
> new_dpcd_buf |= DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
>
> @@ -406,13 +402,6 @@ intel_dp_aux_vesa_enable_backlight(const struct 
> intel_crtc_state *crtc_state,
>pwmgen_bit_count) != 1)
> drm_dbg_kms(&i915->drm,
> "Failed to write aux pwmgen bit count\n");
> -
> -   break;
> -
> -   /* Do nothing when it is already DPCD mode */
> -   case DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD:
> -   default:
> -   break;
> }
>
> if (panel->backlight.edp.vesa.pwm_freq_pre_divider) {
> --
> 2.29.2
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 3/7] drm: Add DisplayPort colorspace property

2019-09-10 Thread Ilia Mirkin
On Tue, Sep 10, 2019 at 9:21 AM Ilia Mirkin  wrote:
>
> On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong
>  wrote:
> >
> > On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote:
> > > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong
> > >  wrote:
> > > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote:
> > > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä
> > > > >  wrote:
> > > > > > On Fri, Sep 06, 2019 at 11:31:55AM +0000, Shankar, Uma wrote:
> > > > > > > > -Original Message-
> > > > > > > > From: Ilia Mirkin 
> > > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM
> > > > > > > > To: Mun, Gwan-gyeong 
> > > > > > > > Cc: Intel Graphics Development <
> > > > > > > > intel-gfx@lists.freedesktop.org
> > > > > > > > > ; Shankar, Uma
> > > > > > > > ; dri-devel <
> > > > > > > > dri-de...@lists.freedesktop.org>
> > > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace
> > > > > > > > property
> > > > > > > >
> > > > > > > > So how would this work with a DP++ connector? Should it
> > > > > > > > list
> > > > > > > > the HDMI or DP
> > > > > > > > properties? Or do we need a custom property checker which
> > > > > > > > is
> > > > > > > > aware of what is
> > > > > > > > currently plugged in to validate the values?
> > > > > > >
> > > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving
> > > > > > > DP
> > > > > > > or HDMI (with a passive dongle).
> > > > > > > Based on the type of sink detected, we should expose DP or
> > > > > > > HDMI
> > > > > > > colorspaces to userspace.
> > > > > >
> > > > > > For i915 DP connector always drives DP mode, HDMI connector
> > > > > > always
> > > > > > drives
> > > > > > HDMI mode, even when the physical connector is DP++.
> > > > >
> > > > > Right, i915 creates 2 connectors, while nouveau, radeon, and
> > > > > amdgpu
> > > > > create 1 connector (not sure about other drivers) for a single
> > > > > physical DP++ socket. Since we supply the list of valid values at
> > > > > the
> > > > > time of creating the connector, we can't know at that point
> > > > > whether
> > > > > in
> > > > > the future a HDMI or DP will be plugged into it.
> > > > >
> > > > >   -ilia
> > > > Ilia, does it mean that the drm_connector type is
> > > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode?
> > >
> > > That is correct. The connector type is "DisplayPort" in such a case.
> > >
> > > Cheers,
> > >
> > >   -ilia
> >
> > For now drm_mode_create_colorspace_property() is only used for i915.
> > IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for
> > using of drm_mode_create_colorspace_property(),
> > what about do we add a variable which can identify DP++ and DP to
> > drm_connector?
> > And when the drivers (nouveau, radeon, and amdgpu) detect the current
> > protocol, the drivers will set the variable.
>
> I've been working on adding this to nouveau.
>
> Can/should such properties be added/removed at "runtime", rather than
> at connector creation time? Either way, the function
> drm_mode_create_colorspace_property as proposed would not be reusable,
> since it looks at the connector type, which will always be
> "DisplayPort" in such cases.

Summary of conversation Ville and I had on IRC:

 - DP++ connectors to provide a single combined list of options for colorspace
 - set_property hook will check against currently plugged in thing,
and reject incorrect values
 - in the case where someone sets e.g. an HDMI value for an
HDMI-plugged-in thing, and then unplugs, and then plugs in a DP
screen, the modeset should continue to succeed but use a default
colorspace value.

I think there was a bit of contention on that last point. Open to
opinions, but we should try to avoid putting undue burden on esp
non-atomic userspace.

Cheers,

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 3/7] drm: Add DisplayPort colorspace property

2019-09-10 Thread Ilia Mirkin
On Tue, Sep 10, 2019 at 3:34 AM Mun, Gwan-gyeong
 wrote:
>
> On Sat, 2019-09-07 at 21:43 -0400, Ilia Mirkin wrote:
> > On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong
> >  wrote:
> > > On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote:
> > > > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä
> > > >  wrote:
> > > > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote:
> > > > > > > -Original Message-
> > > > > > > From: Ilia Mirkin 
> > > > > > > Sent: Tuesday, September 3, 2019 6:12 PM
> > > > > > > To: Mun, Gwan-gyeong 
> > > > > > > Cc: Intel Graphics Development <
> > > > > > > intel-gfx@lists.freedesktop.org
> > > > > > > > ; Shankar, Uma
> > > > > > > ; dri-devel <
> > > > > > > dri-de...@lists.freedesktop.org>
> > > > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace
> > > > > > > property
> > > > > > >
> > > > > > > So how would this work with a DP++ connector? Should it
> > > > > > > list
> > > > > > > the HDMI or DP
> > > > > > > properties? Or do we need a custom property checker which
> > > > > > > is
> > > > > > > aware of what is
> > > > > > > currently plugged in to validate the values?
> > > > > >
> > > > > > AFAIU For DP++ cases, we detect what kind of sink its driving
> > > > > > DP
> > > > > > or HDMI (with a passive dongle).
> > > > > > Based on the type of sink detected, we should expose DP or
> > > > > > HDMI
> > > > > > colorspaces to userspace.
> > > > >
> > > > > For i915 DP connector always drives DP mode, HDMI connector
> > > > > always
> > > > > drives
> > > > > HDMI mode, even when the physical connector is DP++.
> > > >
> > > > Right, i915 creates 2 connectors, while nouveau, radeon, and
> > > > amdgpu
> > > > create 1 connector (not sure about other drivers) for a single
> > > > physical DP++ socket. Since we supply the list of valid values at
> > > > the
> > > > time of creating the connector, we can't know at that point
> > > > whether
> > > > in
> > > > the future a HDMI or DP will be plugged into it.
> > > >
> > > >   -ilia
> > > Ilia, does it mean that the drm_connector type is
> > > DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode?
> >
> > That is correct. The connector type is "DisplayPort" in such a case.
> >
> > Cheers,
> >
> >   -ilia
>
> For now drm_mode_create_colorspace_property() is only used for i915.
> IMHO, when other drivers ( nouveau, radeon, and amdgpu ) are ready for
> using of drm_mode_create_colorspace_property(),
> what about do we add a variable which can identify DP++ and DP to
> drm_connector?
> And when the drivers (nouveau, radeon, and amdgpu) detect the current
> protocol, the drivers will set the variable.

I've been working on adding this to nouveau.

Can/should such properties be added/removed at "runtime", rather than
at connector creation time? Either way, the function
drm_mode_create_colorspace_property as proposed would not be reusable,
since it looks at the connector type, which will always be
"DisplayPort" in such cases.

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [Nouveau] [PATCH v6 08/17] drm/ttm: use gem vma_node

2019-09-07 Thread Ilia Mirkin
On Wed, Aug 21, 2019 at 7:55 AM Thierry Reding  wrote:
>
> On Wed, Aug 21, 2019 at 04:33:58PM +1000, Ben Skeggs wrote:
> > On Wed, 14 Aug 2019 at 20:14, Gerd Hoffmann  wrote:
> > >
> > >   Hi,
> > >
> > > > > Changing the order doesn't look hard.  Patch attached (untested, have 
> > > > > no
> > > > > test hardware).  But maybe I missed some detail ...
> > > >
> > > > I came up with something very similar by splitting up nouveau_bo_new()
> > > > into allocation and initialization steps, so that when necessary the GEM
> > > > object can be initialized in between. I think that's slightly more
> > > > flexible and easier to understand than a boolean flag.
> > >
> > > Yes, that should work too.
> > >
> > > Acked-by: Gerd Hoffmann 
> > Acked-by: Ben Skeggs 
>
> Thanks guys, applied to drm-misc-next.

Hi Thierry,

Initial investigations suggest that this commit currently in drm-next

commit 019cbd4a4feb3aa3a917d78e7110e3011bbff6d5
Author: Thierry Reding 
Date:   Wed Aug 14 11:00:48 2019 +0200

drm/nouveau: Initialize GEM object before TTM object

breaks nouveau userspace which tries to allocate GEM objects with a
non-page-aligned size. Previously nouveau_gem_new would just call
nouveau_bo_init which would call nouveau_bo_fixup_align before
initializing the GEM object. With this change, it is done after. What
do you think -- OK to just move that bit of logic into the new
nouveau_bo_alloc() (and make size/align be pointers so that they can
be fixed up?)

Cheers,

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 3/7] drm: Add DisplayPort colorspace property

2019-09-07 Thread Ilia Mirkin
On Sat, Sep 7, 2019 at 7:20 PM Mun, Gwan-gyeong
 wrote:
>
> On Fri, 2019-09-06 at 09:24 -0400, Ilia Mirkin wrote:
> > On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä
> >  wrote:
> > > On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote:
> > > >
> > > > > -Original Message-
> > > > > From: Ilia Mirkin 
> > > > > Sent: Tuesday, September 3, 2019 6:12 PM
> > > > > To: Mun, Gwan-gyeong 
> > > > > Cc: Intel Graphics Development  > > > > >; Shankar, Uma
> > > > > ; dri-devel <
> > > > > dri-de...@lists.freedesktop.org>
> > > > > Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace
> > > > > property
> > > > >
> > > > > So how would this work with a DP++ connector? Should it list
> > > > > the HDMI or DP
> > > > > properties? Or do we need a custom property checker which is
> > > > > aware of what is
> > > > > currently plugged in to validate the values?
> > > >
> > > > AFAIU For DP++ cases, we detect what kind of sink its driving DP
> > > > or HDMI (with a passive dongle).
> > > > Based on the type of sink detected, we should expose DP or HDMI
> > > > colorspaces to userspace.
> > >
> > > For i915 DP connector always drives DP mode, HDMI connector always
> > > drives
> > > HDMI mode, even when the physical connector is DP++.
> >
> > Right, i915 creates 2 connectors, while nouveau, radeon, and amdgpu
> > create 1 connector (not sure about other drivers) for a single
> > physical DP++ socket. Since we supply the list of valid values at the
> > time of creating the connector, we can't know at that point whether
> > in
> > the future a HDMI or DP will be plugged into it.
> >
> >   -ilia
> Ilia, does it mean that the drm_connector type is
> DRM_MODE_CONNECTOR_DisplayPort and protocol is DP++ mode?

That is correct. The connector type is "DisplayPort" in such a case.

Cheers,

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 3/7] drm: Add DisplayPort colorspace property

2019-09-06 Thread Ilia Mirkin
On Fri, Sep 6, 2019 at 7:43 AM Ville Syrjälä
 wrote:
>
> On Fri, Sep 06, 2019 at 11:31:55AM +, Shankar, Uma wrote:
> >
> >
> > >-Original Message-
> > >From: Ilia Mirkin 
> > >Sent: Tuesday, September 3, 2019 6:12 PM
> > >To: Mun, Gwan-gyeong 
> > >Cc: Intel Graphics Development ; Shankar, 
> > >Uma
> > >; dri-devel 
> > >Subject: Re: [PATCH v4 3/7] drm: Add DisplayPort colorspace property
> > >
> > >So how would this work with a DP++ connector? Should it list the HDMI or DP
> > >properties? Or do we need a custom property checker which is aware of what 
> > >is
> > >currently plugged in to validate the values?
> >
> > AFAIU For DP++ cases, we detect what kind of sink its driving DP or HDMI 
> > (with a passive dongle).
> > Based on the type of sink detected, we should expose DP or HDMI colorspaces 
> > to userspace.
>
> For i915 DP connector always drives DP mode, HDMI connector always drives
> HDMI mode, even when the physical connector is DP++.

Right, i915 creates 2 connectors, while nouveau, radeon, and amdgpu
create 1 connector (not sure about other drivers) for a single
physical DP++ socket. Since we supply the list of valid values at the
time of creating the connector, we can't know at that point whether in
the future a HDMI or DP will be plugged into it.

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v4 3/7] drm: Add DisplayPort colorspace property

2019-09-03 Thread Ilia Mirkin
So how would this work with a DP++ connector? Should it list the HDMI
or DP properties? Or do we need a custom property checker which is
aware of what is currently plugged in to validate the values?

On Tue, Sep 3, 2019 at 5:12 AM Gwan-gyeong Mun
 wrote:
>
> In order to use colorspace property to Display Port connectors, it extends
> DRM_MODE_CONNECTOR_DisplayPort connector_type on
> drm_mode_create_colorspace_property function.
>
> v3: Addressed review comments from Ville
> - Add new colorimetry options for DP 1.4a spec.
> - Separate set of colorimetry enum values for DP.
> v4: Add additional comments to struct drm_prop_enum_list.
> Polishing an enum string of struct drm_prop_enum_list
> Signed-off-by: Gwan-gyeong Mun 
> Reviewed-by: Uma Shankar 
> ---
>  drivers/gpu/drm/drm_connector.c | 46 +
>  include/drm/drm_connector.h |  8 ++
>  2 files changed, 54 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 4c766624b20d..5834e6d330a0 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -882,6 +882,44 @@ static const struct drm_prop_enum_list 
> hdmi_colorspaces[] = {
> { DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER, "DCI-P3_RGB_Theater" },
>  };
>
> +/*
> + * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel 
> Encoding/Colorimetry
> + * Format Table 2-120
> + */
> +static const struct drm_prop_enum_list dp_colorspaces[] = {
> +   /* For Default case, driver will set the colorspace */
> +   { DRM_MODE_COLORIMETRY_DEFAULT, "Default" },
> +   /* Colorimetry based on IEC 61966-2-1 */
> +   { DRM_MODE_COLORIMETRY_SRGB, "sRGB" },
> +   { DRM_MODE_COLORIMETRY_WIDE_GAMUT_FIXED_POINT_RGB, 
> "wide_gamut_fixed_point_RGB" },
> +   /* Colorimetry based on IEC 61966-2-2, wide gamut floating point RGB 
> */
> +   { DRM_MODE_COLORIMETRY_SCRGB, "scRGB" },
> +   { DRM_MODE_COLORIMETRY_ADOBE_RGB, "Adobe_RGB" },
> +   /* Colorimetry based on SMPTE RP 431-2 */
> +   { DRM_MODE_COLORIMETRY_DCP_P3_RGB, "DCI-P3_RGB" },
> +   /* Colorimetry based on ITU-R BT.2020 */
> +   { DRM_MODE_COLORIMETRY_BT2020_RGB, "BT2020_RGB" },
> +   { DRM_MODE_COLORIMETRY_BT601_YCC, "BT601_YCC" },
> +   { DRM_MODE_COLORIMETRY_BT709_YCC, "BT709_YCC" },
> +   /* Standard Definition Colorimetry based on IEC 61966-2-4 */
> +   { DRM_MODE_COLORIMETRY_XVYCC_601, "XVYCC_601" },
> +   /* High Definition Colorimetry based on IEC 61966-2-4 */
> +   { DRM_MODE_COLORIMETRY_XVYCC_709, "XVYCC_709" },
> +   /* Colorimetry based on IEC 61966-2-1/Amendment 1 */
> +   { DRM_MODE_COLORIMETRY_SYCC_601, "SYCC_601" },
> +   /* Colorimetry based on IEC 61966-2-5 [33] */
> +   { DRM_MODE_COLORIMETRY_OPYCC_601, "opYCC_601" },
> +   /* Colorimetry based on ITU-R BT.2020 */
> +   { DRM_MODE_COLORIMETRY_BT2020_CYCC, "BT2020_CYCC" },
> +   /* Colorimetry based on ITU-R BT.2020 */
> +   { DRM_MODE_COLORIMETRY_BT2020_YCC, "BT2020_YCC" },
> +   /*
> +* Colorumetry based on Digital Imaging and Communications in Medicine
> +* (DICOM) Part 14: Grayscale Standard Display Function
> +*/
> +   { DRM_MODE_COLORIMETRY_DICOM_PART_14_GRAYSCALE, 
> "DICOM_Part_14_Grayscale" },
> +};
> +
>  /**
>   * DOC: standard connector properties
>   *
> @@ -1710,6 +1748,14 @@ int drm_mode_create_colorspace_property(struct 
> drm_connector *connector)
> ARRAY_SIZE(hdmi_colorspaces));
> if (!prop)
> return -ENOMEM;
> +   } else if (connector->connector_type == 
> DRM_MODE_CONNECTOR_DisplayPort ||
> +  connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +   prop = drm_property_create_enum(dev, DRM_MODE_PROP_ENUM,
> +   "Colorspace",
> +   dp_colorspaces,
> +   ARRAY_SIZE(dp_colorspaces));
> +   if (!prop)
> +   return -ENOMEM;
> } else {
> DRM_DEBUG_KMS("Colorspace property not supported\n");
> return 0;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 681cb590f952..8848e5d6b0c4 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -281,6 +281,14 @@ enum drm_panel_orientation {
>  /* Additional Colorimetry extension added as part of CTA 861.G */
>  #define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D6511
>  #define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER12
> +/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
> +#define DRM_MODE_COLORIMETRY_SRGB  13
> +#define DRM_MODE_COLORIMETRY_WIDE_GAMUT_FIXED_POINT_RGB14
> +#define DRM_MODE_COLORIMETRY_SCRGB 15

Re: [Intel-gfx] [PATCH] drm: Dump mode picture aspect ratio

2019-06-20 Thread Ilia Mirkin
On Thu, Jun 20, 2019 at 11:46 AM Ville Syrjala
 wrote:
>
> From: Ville Syrjälä 
>
> Currently the logs show nothing about the mode's aspect ratio.
> Include that information in the normal mode dump.
>
> Cc: Ilia Mirkin 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/video/hdmi.c| 3 ++-
>  include/drm/drm_modes.h | 4 ++--
>  include/linux/hdmi.h| 3 +++
>  3 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c
> index b939bc28d886..bc593fe1c268 100644
> --- a/drivers/video/hdmi.c
> +++ b/drivers/video/hdmi.c
> @@ -1057,7 +1057,7 @@ static const char *hdmi_colorimetry_get_name(enum 
> hdmi_colorimetry colorimetry)
> return "Invalid";
>  }
>
> -static const char *
> +const char *
>  hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect)
>  {
> switch (picture_aspect) {
> @@ -1076,6 +1076,7 @@ hdmi_picture_aspect_get_name(enum hdmi_picture_aspect 
> picture_aspect)
> }
> return "Invalid";
>  }
> +EXPORT_SYMBOL(hdmi_picture_aspect_get_name);

So this will return "No Data" most of the time (since the DRM_CAP
won't be enabled)? This will look awkward, esp since the person seeing
this print will have no idea what "No Data" is referring to.

>
>  static const char *
>  hdmi_active_aspect_get_name(enum hdmi_active_aspect active_aspect)
> diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h
> index 083f16747369..2b1809c74fbe 100644
> --- a/include/drm/drm_modes.h
> +++ b/include/drm/drm_modes.h
> @@ -431,7 +431,7 @@ struct drm_display_mode {
>  /**
>   * DRM_MODE_FMT - printf string for &struct drm_display_mode
>   */
> -#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x"
> +#define DRM_MODE_FMT"\"%s\": %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x %s"
>
>  /**
>   * DRM_MODE_ARG - printf arguments for &struct drm_display_mode
> @@ -441,7 +441,7 @@ struct drm_display_mode {
> (m)->name, (m)->vrefresh, (m)->clock, \
> (m)->hdisplay, (m)->hsync_start, (m)->hsync_end, (m)->htotal, \
> (m)->vdisplay, (m)->vsync_start, (m)->vsync_end, (m)->vtotal, \
> -   (m)->type, (m)->flags
> +   (m)->type, (m)->flags, 
> hdmi_picture_aspect_get_name((m)->picture_aspect_ratio)

Flags are printed as a literal integer value. Given that AR stuff is
fairly esoteric, I might just print an integer here too. (Why was
picture_aspect_ratio not stuffed into ->flags in the first place?)

>
>  #define obj_to_mode(x) container_of(x, struct drm_display_mode, base)
>
> diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h
> index 9918a6c910c5..de7cbe385dba 100644
> --- a/include/linux/hdmi.h
> +++ b/include/linux/hdmi.h
> @@ -434,4 +434,7 @@ int hdmi_infoframe_unpack(union hdmi_infoframe *frame,
>  void hdmi_infoframe_log(const char *level, struct device *dev,
> const union hdmi_infoframe *frame);
>
> +const char *
> +hdmi_picture_aspect_get_name(enum hdmi_picture_aspect picture_aspect);
> +
>  #endif /* _DRM_HDMI_H */
> --
> 2.21.0
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/dp: Correctly mask DP_TRAINING_AUX_RD_INTERVAL values for DP 1.4

2018-03-07 Thread Ilia Mirkin
On Wed, Mar 7, 2018 at 6:44 PM,   wrote:
> From: Matt Atwood 
>
> DP_TRAINING_AUX_RD_INTERVAL with DP 1.3 spec changed bit scheeme from 8
> bits to 7 in DPCD 0x000e. The 8th bit is used to identify extended
> receiver capabilities. For panels that use this new feature wait interval
> would be increased by 512 ms, when spec is max 16 ms. This behavior is
> described in table 2-158 of DP 1.4 spec address eh.
>
> With the introduction of DP 1.4 spec main link clock recovery was
> standardized to 100 us regardless of TRAINING_AUX_RD_INTERVAL value.
>
> To avoid breaking panels that are not spec compiant we now warn on
> invalid values.
>
> V2: commit title/message, masking all 7 bits, warn on out of spec values.
> V3: commit message, make link train clock recovery follow DP 1.4 spec.
>
> Signed-off-by: Matt Atwood 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 18 ++
>  include/drm/drm_dp_helper.h |  4 
>  2 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index adf79be..671b823 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -119,18 +119,28 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 
> link_status[DP_LINK_STATUS_SI
>  EXPORT_SYMBOL(drm_dp_get_adjust_request_pre_emphasis);
>
>  void drm_dp_link_train_clock_recovery_delay(const u8 
> dpcd[DP_RECEIVER_CAP_SIZE]) {
> -   if (dpcd[DP_TRAINING_AUX_RD_INTERVAL] == 0)
> +   int rd_interval = dpcd[DP_TRAINING_AUX_RD_INTERVAL] & 
> DP_TRAINING_AUX_RD_MASK;
> +
> +   if (rd_interval > 4)
> +   DRM_DEBUG_KMS("AUX interval %d, out of range (max 4)", 
> rd_interval);
> +
> +   if(rd_interval == 0 || (dpcd[DP_DPCD_REV] & DP_REV_14))

Was this meant to be dpcd[DP_DPCD_REV] >= DP_REV_14? It doesn't appear
to be a bitmask...

Also I think you're supposed to say "if (" rather than "if(".

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Nouveau] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr

2017-07-13 Thread Ilia Mirkin
On Thu, Jul 13, 2017 at 10:14 PM, Michel Dänzer  wrote:
> On 13/07/17 09:31 PM, Ilia Mirkin wrote:
>> On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer  wrote:
>>> On 18/04/17 07:07 PM, Michel Dänzer wrote:
>>>> From: Michel Dänzer 
>>>>
>>>> Signed-off-by: Michel Dänzer 
>>>> ---
>>>>
>>>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau
>>>> drivers, right?
>>>
>>> Any feedback, guys?
>>>
>>> I want to push the xserver change soon-ish. I could probably come up
>>> with a corresponding patch for nouveau in the worst case, but I'm afraid
>>> not for intel.
>>
>> Sorry, I'm not 1000% clear on what this patch is doing.
>
> 100% should suffice. ;)
>
>> Is it literally just a type change from A to B and so code has to be fixed
>> up for the new API?
>
> That's basically it, corresponding to
> https://patchwork.freedesktop.org/patch/150938/ .

From a brief glance at nouveau, it never uses PixmapDirtyUpdate,
except in src/nv_driver.c where I don't think the code would need to
be updated.

https://cgit.freedesktop.org/nouveau/xf86-video-nouveau/tree/src/nv_driver.c#n552

Let me know if you think I'm off.

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [Nouveau] [PATCH xf86-video-amdgpu] Adapt to PixmapDirtyUpdateRec::src being a DrawablePtr

2017-07-13 Thread Ilia Mirkin
On Thu, Jul 13, 2017 at 4:27 AM, Michel Dänzer  wrote:
> On 18/04/17 07:07 PM, Michel Dänzer wrote:
>> From: Michel Dänzer 
>>
>> Signed-off-by: Michel Dänzer 
>> ---
>>
>> Chris / Ilia / Ben, this should be manageable for the intel/nouveau
>> drivers, right?
>
> Any feedback, guys?
>
> I want to push the xserver change soon-ish. I could probably come up
> with a corresponding patch for nouveau in the worst case, but I'm afraid
> not for intel.

Sorry, I'm not 1000% clear on what this patch is doing. Is it
literally just a type change from A to B and so code has to be fixed
up for the new API? Or are there further implications like having to
do/support new things?

Thanks,

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames

2017-03-23 Thread Ilia Mirkin
On Thu, Mar 23, 2017 at 11:22 AM, Sharma, Shashank
 wrote:
> On 3/23/2017 5:17 PM, Ilia Mirkin wrote:
>>
>> On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma
>>  wrote:
>>>
>>> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC
>>> 1-64).
>>> For any other mode, the VIC filed in AVI infoframes should be 0.
>>> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
>>> extended to (VIC 1-107).
>>>
>>> This patch adds a bool input variable, which indicates if the connected
>>> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
>>> HDMI 2.0 VIC to a HDMI 1.4 sink.
>>
>> Should this patch come before the patch which recognizes VICs 65+?
>> Otherwise if only the first patch is applied but not this one, you
>> could end up with the bad scenario. (As can happen in bisections, for
>> example.)
>
> I kindof agree, this could be the case, but also, for the correct
> functionality, you should have the whole series
> together.

OK, so ... I have a bug in my filesystem, and I'm bisecting it and
rebooting my box at each step. I happen to hit this commit in my
bisect and my monitor doesn't turn on? Seems unpleasant, no?
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4 2/2] drm: Add HDMI 2.0 VIC support for AVI info-frames

2017-03-23 Thread Ilia Mirkin
On Thu, Mar 23, 2017 at 11:14 AM, Shashank Sharma
 wrote:
> HDMI 1.4b support the CEA video modes as per range of CEA-861-D (VIC 1-64).
> For any other mode, the VIC filed in AVI infoframes should be 0.
> HDMI 2.0 sinks, support video modes range as per CEA-861-F spec, which is
> extended to (VIC 1-107).
>
> This patch adds a bool input variable, which indicates if the connected
> sink is a HDMI 2.0 sink or not. This will make sure that we don't pass a
> HDMI 2.0 VIC to a HDMI 1.4 sink.

Should this patch come before the patch which recognizes VICs 65+?
Otherwise if only the first patch is applied but not this one, you
could end up with the bad scenario. (As can happen in bisections, for
example.)

>
> This patch touches all drm drivers, who are callers of this function
> drm_hdmi_avi_infoframe_from_display_mode but to make sure there is
> no change in current behavior, is_hdmi2 is kept as false.
>
> In case of I915 driver, this patch checks the connector->display_info
> to check if the connected display is HDMI 2.0.
>
> Cc: Ville Syrjala 
> Cc: Jose Abreu 
> Cc: Andrzej Hajda 
> Cc: Alex Deucher 
> Cc: Daniel Vetter 
>
> PS: This patch touches a few lines in few files, which were
> already above 80 char, so checkpatch gives 80 char warning again.
> - gpu/drm/omapdrm/omap_encoder.c
> - gpu/drm/i915/intel_sdvo.c
>
> Signed-off-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c|  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c |  2 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  3 ++-
>  drivers/gpu/drm/bridge/sii902x.c  |  2 +-
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  2 +-
>  drivers/gpu/drm/drm_edid.c| 12 +++-
>  drivers/gpu/drm/exynos/exynos_hdmi.c  |  2 +-
>  drivers/gpu/drm/i2c/tda998x_drv.c |  2 +-
>  drivers/gpu/drm/i915/intel_hdmi.c |  5 -
>  drivers/gpu/drm/i915/intel_sdvo.c |  3 ++-
>  drivers/gpu/drm/mediatek/mtk_hdmi.c   |  2 +-
>  drivers/gpu/drm/omapdrm/omap_encoder.c|  3 ++-
>  drivers/gpu/drm/radeon/radeon_audio.c |  2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c  |  2 +-
>  drivers/gpu/drm/sti/sti_hdmi.c|  2 +-
>  drivers/gpu/drm/tegra/hdmi.c  |  2 +-
>  drivers/gpu/drm/tegra/sor.c   |  2 +-
>  drivers/gpu/drm/vc4/vc4_hdmi.c|  2 +-
>  drivers/gpu/drm/zte/zx_hdmi.c |  2 +-
>  include/drm/drm_edid.h|  3 ++-
>  21 files changed, 38 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index d4452d8..ab7a399 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1877,7 +1877,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder 
> *encoder,
> dce_v10_0_audio_write_sad_regs(encoder);
> dce_v10_0_audio_write_latency_fields(encoder, mode);
>
> -   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> +   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 5b24e89..3c5fd83 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1861,7 +1861,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder 
> *encoder,
> dce_v11_0_audio_write_sad_regs(encoder);
> dce_v11_0_audio_write_latency_fields(encoder, mode);
>
> -   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> +   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index d2590d7..c564dca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1760,7 +1760,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder 
> *encoder,
> dce_v8_0_audio_write_sad_regs(encoder);
> dce_v8_0_audio_write_latency_fields(encoder, mode);
>
> -   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> +   err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> if (err < 0) {
> DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> return;
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c 
> b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> index a2a8236..f9b77b8 100644
> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> @@ -1097,7 +1097,8 @@ static void anx78xx_bridge_mode_set(struct drm_bridge 
> *bridg

Re: [Intel-gfx] [PATCH 07/10] drm/i915/psr: set PSR_MASK bits for deep sleep

2017-01-03 Thread Ilia Mirkin
On Mon, Jan 2, 2017 at 6:31 AM, vathsala nagaraju
 wrote:
> Program EDP_PSR_DEBUG_CTL (PSR_MASK) to enable system
> to go to deep sleep while in psr2.PSR2_STATUS bit 31:28
> should report value 8 , if system enters deep sleep state.
>
> Also, EDP_FRAMES_BEFORE_SU_ENTRY is set 1 , if not set,
> flickering is observed on psr2 panel.
>
> Cc: Rodrigo Vivi 
> Cc: Jim Bride 
> Signed-off-by: Vathsala Nagaraju 
> Signed-off-by: Patil Deepti 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  7 +++
>  drivers/gpu/drm/i915/intel_dp.c  |  1 -
>  drivers/gpu/drm/i915/intel_psr.c | 29 -
>  3 files changed, 27 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 5ca506a..0cbe564 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3600,6 +3600,12 @@ enum {
>  #define   EDP_PSR_DEBUG_MASK_LPSP  (1<<27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP (1<<26)
>  #define   EDP_PSR_DEBUG_MASK_HPD   (1<<25)
> +#define   EDP_PSR_DEBUG_MASK_MAX_SLEEP (1<<28)
> +#define   EDP_PSR_DEBUG_MASK_LPSP  (1<<27)
> +#define   EDP_PSR_DEBUG_MASK_MEMUP (1<<26)
> +#define   EDP_PSR_DEBUG_MASK_HPD   (1<<25)

Looks like you're defining the above 3 (maybe 4 - not enough context)
a second time.

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/3] drm: Add DP port types from DP 1.3 specification

2016-05-03 Thread Ilia Mirkin
On May 3, 2016 9:49 AM, "Mika Kahola"  wrote:
>
> DP specification 1.3 defines DP downstream ports for
> DP++ and wireless devices. Let's add these to port
> definitions.
>
> Signed-off-by: Mika Kahola 
> ---
>  include/drm/drm_dp_helper.h | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 92d9a52..9a15099 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -210,6 +210,8 @@
>  # define DP_DS_PORT_TYPE_DVI   2
>  # define DP_DS_PORT_TYPE_HDMI  3
>  # define DP_DS_PORT_TYPE_NON_EDID  4
> +# define DP_DP_PORT_TYPE_DP_DUALMODE5

DP_DS right? (Like all the others)

> +# define DP_DS_PORT_TYPE_WIRELESS   6
>  # define DP_DS_PORT_HPD(1 << 3)
>  /* offset 1 for VGA is maximum megapixels per second / 8 */
>  /* offset 2 */
> --
> 1.9.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 4/5] drm/atomic-helper: Reject legacy flips on a disabled pipe

2015-12-08 Thread Ilia Mirkin
On Tue, Dec 8, 2015 at 3:49 AM, Daniel Vetter  wrote:
> We want this for consistency with existing page_flip semantics.
>
> Since this spurred quite a discussion on IRC also document why we
> reject even generation when the pipe is off: It's not that it's hard

event generation?

> to implement, but userspace has a track recording proofing that it's

has a track record which proves that it's

> way too easy to accidentally abuse and cause havoc. We want to make
> sure userspace doesn't get away with that.
>
> v2: Somehow thought we do reject events already, but that code only
> existed in my imagination ... Also suggestions from Thierry.
>
> Cc: Daniel Stone 
> Cc: Ville Syrjälä 
> Cc: Thierry Reding 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_atomic.c| 16 
>  drivers/gpu/drm/drm_atomic_helper.c |  9 +
>  include/drm/drm_crtc.h  |  3 ++-
>  3 files changed, 27 insertions(+), 1 deletion(-)
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 13/28] drm/nouveau: Use private save/restore hooks

2015-12-04 Thread Ilia Mirkin
On Fri, Dec 4, 2015 at 3:45 AM, Daniel Vetter  wrote:
> I want to remove the core ones since with atomic drivers system
> suspend/resume is solved much differently. And there's only 2 drivers
> (gma500 besides nouveau) really using them.
>
> Cc: Ben Skeggs 
> Signed-off-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c |  5 +++--
>  drivers/gpu/drm/nouveau/dispnv04/disp.c | 11 ++-
>  drivers/gpu/drm/nouveau/nouveau_crtc.h  |  3 +++
>  3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c 
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index dab24066fa21..bb9e9cb14b9d 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -1081,8 +1081,6 @@ nouveau_crtc_set_config(struct drm_mode_set *set)
>  }
>
>  static const struct drm_crtc_funcs nv04_crtc_funcs = {
> -   .save = nv_crtc_save,
> -   .restore = nv_crtc_restore,
> .cursor_set = nv04_crtc_cursor_set,
> .cursor_move = nv04_crtc_cursor_move,
> .gamma_set = nv_crtc_gamma_set,
> @@ -1123,6 +1121,9 @@ nv04_crtc_create(struct drm_device *dev, int crtc_num)
> nv_crtc->index = crtc_num;
> nv_crtc->last_dpms = NV_DPMS_CLEARED;
>
> +   nv_crtc->save = nv_crtc_save;
> +   nv_crtc->restore = nv_crtc_restore;
> +
> drm_crtc_init(dev, &nv_crtc->base, &nv04_crtc_funcs);
> drm_crtc_helper_add(&nv_crtc->base, &nv04_crtc_helper_funcs);
> drm_mode_crtc_set_gamma_size(&nv_crtc->base, 256);
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index 9e650081c357..ebd9430e0628 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -39,7 +39,7 @@ nv04_display_create(struct drm_device *dev)
> struct dcb_table *dcb = &drm->vbios.dcb;
> struct drm_connector *connector, *ct;
> struct drm_encoder *encoder;
> -   struct drm_crtc *crtc;
> +   struct nouveau_crtc *crtc;
> struct nv04_display *disp;
> int i, ret;
>
> @@ -107,8 +107,8 @@ nv04_display_create(struct drm_device *dev)
> }
>
> /* Save previous state */
> -   list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> -   crtc->funcs->save(crtc);
> +   list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head)
> +   crtc->save(crtc);

Won't this warn about incompatible types? (function wants drm_crtc,
but you're giving it nouveau_crtc)?

>
> list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
> const struct drm_encoder_helper_funcs *func = 
> encoder->helper_private;
> @@ -128,6 +128,7 @@ nv04_display_destroy(struct drm_device *dev)
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct drm_encoder *encoder;
> struct drm_crtc *crtc;
> +   struct nouveau_crtc *nv_crtc;
>
> /* Turn every CRTC off. */
> list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
> @@ -145,8 +146,8 @@ nv04_display_destroy(struct drm_device *dev)
> func->restore(encoder);
> }
>
> -   list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
> -   crtc->funcs->restore(crtc);
> +   list_for_each_entry(nv_crtc, &dev->mode_config.crtc_list, base.head)
> +   nv_crtc->restore(crtc);

Why is this OK? Don't you want to use nv_crtc here as the function argument?

>
> nouveau_hw_save_vga_fonts(dev, 0);
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h 
> b/drivers/gpu/drm/nouveau/nouveau_crtc.h
> index f19cb1c5fc5a..863f10b8d818 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
> @@ -73,6 +73,9 @@ struct nouveau_crtc {
> int (*set_dither)(struct nouveau_crtc *crtc, bool update);
> int (*set_scale)(struct nouveau_crtc *crtc, bool update);
> int (*set_color_vibrance)(struct nouveau_crtc *crtc, bool update);
> +
> +   void (*save)(struct drm_crtc *crtc);
> +   void (*restore)(struct drm_crtc *crtc);
>  };
>
>  static inline struct nouveau_crtc *nouveau_crtc(struct drm_crtc *crtc)
> --
> 2.5.1
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH RESEND 2/2] drm/i915: prevent the vgacon from ever reloading

2015-11-27 Thread Ilia Mirkin
On Fri, Nov 27, 2015 at 10:40 AM, Emil Velikov  wrote:
> On 27 November 2015 at 15:10, Daniel Vetter  wrote:
>> It only leads to bloodshed and tears - we don't bother to restore a
>> working legacy vga hw setup.
>>
>> On haswell with the new dynamic power well code this leads to even
>> more hilarity since for some configurations the hardware is simply no
>> longer there.
>>
>> The actual implementation is a bit a hack - we realy on fbcon to kick
>> out the vgacon. To make this also work with I915_FBDEV=n (or FBCON=n)
>> and VGA_CONSOLE=y i915 already unregisters the vga console manually
>> early in the driver load sequence.
>>
> Interesting... nv50 and later GPUs are in a roughly similar shame
> afaict. They lack the dedicated hardware and no one really bothered
> figuring out how to restore things to a working shape [1].
>
> Then again, upon sequential load of the nouveau module the GPU gets
> initialised properly, where you can get X (weston?) up and running
> without issues. Am I thinking about a different thing ?
>
> I take it that you guys do less of the (re)initialisation dance, to
> ensure faster boot times ?
>
> Thanks,
> Emil
>
> [1] 
> http://nouveau.freedesktop.org/wiki/KernelModeSetting/#deactivatingkmsandunloadingnouveau

FWIW it's possible to unload nouveau, at which point your screen turns
off (because we can't restore the vga emulation junk... haven't the
faintest clue how it works), and then reload it, at which point fbcon
gets rebound and is completely happy. I can't tell what this patch is
doing, but for your own sanity, you should support unloading/reloading
i915 as well.

  -ilia
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH mesa v3] i965/gen8+: bo in state base address must be in 32-bit address range

2015-08-07 Thread Ilia Mirkin
On Fri, Aug 7, 2015 at 5:45 AM, Michel Thierry  wrote:
> Gen8+ supports 48-bit virtual addresses, but some objects must always be
> allocated inside the 32-bit address range.
>
> In specific, any resource used with flat/heapless (0x-0xf000)
> General State Heap or Intruction State Heap must be in a 32-bit range
> (GSH / ISH), because the General State Offset and Instruction State Offset
> are limited to 32-bits.
>
> Use drm_intel_bo_emit_reloc_48bit when the 4GB limit is not necessary, and
> the bo can be in the full address space.
>
> This commit introduces a dependency of libdrm 2.4.63, which introduces the
> drm_intel_bo_emit_reloc_48bit function.
>
> v2: s/48baddress/48b_address/,
> Only use in OUT_RELOC64 cases, OUT_RELOC implies a 32-bit address offset
> is needed (Ben)
> v3: Added OUT_RELOC64_INSIDE_4G, so it stands out when a 64-bit relocation
> needs the 32-bit workaround (Chris)
>
> References: 
> http://lists.freedesktop.org/archives/intel-gfx/2015-July/072612.html
> Cc: Ben Widawsky 
> Cc: Chris Wilson 
> Signed-off-by: Michel Thierry 
> ---
>  configure.ac  |  2 +-
>  src/mesa/drivers/dri/i965/gen8_misc_state.c   | 19 +++
>  src/mesa/drivers/dri/i965/intel_batchbuffer.c | 20 
>  src/mesa/drivers/dri/i965/intel_batchbuffer.h | 10 --
>  4 files changed, 36 insertions(+), 15 deletions(-)
>
> diff --git a/configure.ac b/configure.ac
> index af61aa2..c92ca44 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -68,7 +68,7 @@ AC_SUBST([OSMESA_VERSION])
>  dnl Versions for external dependencies
>  LIBDRM_REQUIRED=2.4.38
>  LIBDRM_RADEON_REQUIRED=2.4.56
> -LIBDRM_INTEL_REQUIRED=2.4.60
> +LIBDRM_INTEL_REQUIRED=2.4.63

There is no such version. I think you need a release before you can
commit this. Otherwise you'll cause pain for a whole lot of people.

>  LIBDRM_NVVIEUX_REQUIRED=2.4.33
>  LIBDRM_NOUVEAU_REQUIRED="2.4.33 libdrm >= 2.4.41"
>  LIBDRM_FREEDRENO_REQUIRED=2.4.57
> diff --git a/src/mesa/drivers/dri/i965/gen8_misc_state.c 
> b/src/mesa/drivers/dri/i965/gen8_misc_state.c
> index b20038e..73eba06 100644
> --- a/src/mesa/drivers/dri/i965/gen8_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_misc_state.c
> @@ -28,6 +28,10 @@
>
>  /**
>   * Define the base addresses which some state is referenced from.
> + *
> + * Use OUT_RELOC64_INSIDE_4G instead of OUT_RELOC64, the General State
> + * Offset and Instruction State Offset are limited to 32-bits by hardware,
> + * and must be located in the first 4GBs (32-bit offset).
>   */
>  void gen8_upload_state_base_address(struct brw_context *brw)
>  {
> @@ -41,19 +45,18 @@ void gen8_upload_state_base_address(struct brw_context 
> *brw)
> OUT_BATCH(0);
> OUT_BATCH(mocs_wb << 16);
> /* Surface state base address: */
> -   OUT_RELOC64(brw->batch.bo, I915_GEM_DOMAIN_SAMPLER, 0,
> -   mocs_wb << 4 | 1);
> +   OUT_RELOC64_INSIDE_4G(brw->batch.bo, I915_GEM_DOMAIN_SAMPLER, 0,
> + mocs_wb << 4 | 1);
> /* Dynamic state base address: */
> -   OUT_RELOC64(brw->batch.bo,
> -   I915_GEM_DOMAIN_RENDER | I915_GEM_DOMAIN_INSTRUCTION, 0,
> -   mocs_wb << 4 | 1);
> +   OUT_RELOC64_INSIDE_4G(brw->batch.bo,
> + I915_GEM_DOMAIN_RENDER | 
> I915_GEM_DOMAIN_INSTRUCTION, 0,
> + mocs_wb << 4 | 1);
> /* Indirect object base address: MEDIA_OBJECT data */
> OUT_BATCH(mocs_wb << 4 | 1);
> OUT_BATCH(0);
> /* Instruction base address: shader kernels (incl. SIP) */
> -   OUT_RELOC64(brw->cache.bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> -   mocs_wb << 4 | 1);
> -
> +   OUT_RELOC64_INSIDE_4G(brw->cache.bo, I915_GEM_DOMAIN_INSTRUCTION, 0,
> + mocs_wb << 4 | 1);
> /* General state buffer size */
> OUT_BATCH(0xf001);
> /* Dynamic state buffer size */
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c 
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 54081a1..ca90784 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -409,11 +409,23 @@ bool
>  intel_batchbuffer_emit_reloc64(struct brw_context *brw,
> drm_intel_bo *buffer,
> uint32_t read_domains, uint32_t write_domain,
> -  uint32_t delta)
> +   uint32_t delta,
> +   bool support_48bit_offset)
>  {
> -   int ret = drm_intel_bo_emit_reloc(brw->batch.bo, 4*brw->batch.used,
> - buffer, delta,
> - read_domains, write_domain);
> +   int ret;
> +
> +   /* Not all buffers can be allocated outside the first 4GB, and
> +* offset must be limited to 32-bits.
> +*/
> +   if (support_48bit_offset)
> +  drm_intel_bo_emit_reloc_48bit(brw->batch.bo, 4*brw->batch.

Re: [Intel-gfx] [PULL] topic/core-stuff

2014-09-24 Thread Ilia Mirkin
On Wed, Sep 24, 2014 at 6:24 AM, Daniel Vetter  wrote:
> Hi Dave,
>
> Just noticed that you've picked up the header rework stuff already, so
> I've rebased that out again. Otherwise just two stragglers from the vblank
> rework and the universal cursor planes locking fix. Plus sprinkling
> container_of all over fbdev emulation from Fabian.

I thought it was questionable whether these were desired... the
pattern of casting between structs that contain others as bases is a
pretty common one, and container_of just makes it harder to read. Did
the discussion end in a way that concluded the opposite? I might have
missed it.

  -ilia

>
> Aside: I only have just 1 fix for drm-next atm for i915 and not terribly
> serious. So will probably only send you a pull for it when the merge
> window opens or something more serious shows up.
>
> Cheers, Daniel
>
>
> The following changes since commit d743ecf360637d489a3ba81a268f574359149601:
>
>   drm/doc: Fixup drm_irq kerneldoc includes. (2014-09-24 11:43:47 +1000)
>
> are available in the git repository at:
>
>   git://anongit.freedesktop.org/drm-intel tags/topic/core-stuff-2014-09-24
>
> for you to fetch changes up to ee0d68ab5f0997a500fdf90924a58e787b216292:
>
>   drm/udl: use container_of to resolve udl_fbdev from drm_fb_helper 
> (2014-09-24 12:09:28 +0200)
>
> 
> Daniel Vetter (2):
>   drm: Fixup locking for universal cursor planes
>   drm: Improve debug output for drm_wait_one_vblank
>
> Fabian Frederick (9):
>   drm/cirrus: use container_of to resolve cirrus_fbdev from drm_fb_helper
>   drm/mgag200: use container_of to resolve mga_fbdev from drm_fb_helper
>   drm/radeon: use container_of to resolve radeon_fbdev from drm_fb_helper
>   drm/nouveau: use container_of to resolve nouveau_fbdev from 
> drm_fb_helper
>   drm/nouveau: use container_of to resolve nouveau_plane from drm_plane
>   drm/qxl: use container_of to resolve qxl_fbdev from drm_fb_helper
>   drm/gma500: use container_of to resolve psb_fbdev from drm_fb_helper
>   drm/ast: use container_of to resolve ast_fbdev from drm_fb_helper
>   drm/udl: use container_of to resolve udl_fbdev from drm_fb_helper
>
> Mario Kleiner (1):
>   drm: Don't update vblank timestamp when the counter didn't change
>
>  drivers/gpu/drm/ast/ast_fb.c   |  3 +-
>  drivers/gpu/drm/cirrus/cirrus_fbdev.c  |  3 +-
>  drivers/gpu/drm/drm_crtc.c | 51 
> --
>  drivers/gpu/drm/drm_irq.c  |  7 ++--
>  drivers/gpu/drm/gma500/framebuffer.c   |  3 +-
>  drivers/gpu/drm/mgag200/mgag200_fb.c   |  3 +-
>  drivers/gpu/drm/nouveau/dispnv04/overlay.c | 15 ++---
>  drivers/gpu/drm/nouveau/nouveau_fbcon.c|  3 +-
>  drivers/gpu/drm/qxl/qxl_fb.c   |  3 +-
>  drivers/gpu/drm/radeon/radeon_fb.c |  3 +-
>  drivers/gpu/drm/udl/udl_fb.c   |  3 +-
>  11 files changed, 65 insertions(+), 32 deletions(-)
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH 2/7] drm/nouveau: replace drm_get_connector_name() with direct name field use

2014-05-27 Thread Ilia Mirkin
On Mon, May 26, 2014 at 9:35 AM, Jani Nikula  wrote:
> Generated using semantic patch:
>
> @@
> expression E;
> @@
>
> - drm_get_connector_name(E)
> + E->name
>
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/nouveau/dispnv04/dac.c  | 2 +-
>  drivers/gpu/drm/nouveau/dispnv04/dfp.c  | 2 +-
>  drivers/gpu/drm/nouveau/dispnv04/disp.c | 2 +-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv04.c   | 3 ++-
>  drivers/gpu/drm/nouveau/dispnv04/tvnv17.c   | 3 +--
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 8 
>  drivers/gpu/drm/nouveau/nv50_display.c  | 2 +-
>  7 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dac.c 
> b/drivers/gpu/drm/nouveau/dispnv04/dac.c
> index 434b920f6bd4..d4d95df2b3c6 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/dac.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/dac.c
> @@ -414,7 +414,7 @@ static void nv04_dac_commit(struct drm_encoder *encoder)
> helper->dpms(encoder, DRM_MODE_DPMS_ON);
>
> NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
> -
> drm_get_connector_name(&nouveau_encoder_connector_get(nv_encoder)->base),
> +(&nouveau_encoder_connector_get(nv_encoder)->base)->name,

That looks pretty vile... how about

nouveau_encoder_connector_get(nv_encoder)->base.name

Here and below.

>  nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
>  }
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/dfp.c 
> b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> index a2d669b4acf2..0615efde05ad 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/dfp.c
> @@ -477,7 +477,7 @@ static void nv04_dfp_commit(struct drm_encoder *encoder)
> helper->dpms(encoder, DRM_MODE_DPMS_ON);
>
> NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
> -
> drm_get_connector_name(&nouveau_encoder_connector_get(nv_encoder)->base),
> +(&nouveau_encoder_connector_get(nv_encoder)->base)->name,
>  nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
>  }
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index 2f1ed61f7c8c..4342fdaee707 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -115,7 +115,7 @@ nv04_display_create(struct drm_device *dev)
>  &dev->mode_config.connector_list, head) {
> if (!connector->encoder_ids[0]) {
> NV_WARN(drm, "%s has no encoders, removing\n",
> -   drm_get_connector_name(connector));
> +   connector->name);
> connector->funcs->destroy(connector);
> }
> }
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c 
> b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> index 244822df8ffc..6b13e1d3c570 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv04.c
> @@ -171,7 +171,8 @@ static void nv04_tv_commit(struct drm_encoder *encoder)
> helper->dpms(encoder, DRM_MODE_DPMS_ON);
>
> NV_DEBUG(drm, "Output %s is running on CRTC %d using output %c\n",
> -
> drm_get_connector_name(&nouveau_encoder_connector_get(nv_encoder)->base), 
> nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
> +(&nouveau_encoder_connector_get(nv_encoder)->base)->name,
> +nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
>  }
>
>  static void nv04_tv_destroy(struct drm_encoder *encoder)
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c 
> b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> index acef48f4a4ea..9026ab97098f 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/tvnv17.c
> @@ -612,8 +612,7 @@ static void nv17_tv_commit(struct drm_encoder *encoder)
> helper->dpms(encoder, DRM_MODE_DPMS_ON);
>
> NV_INFO(drm, "Output %s is running on CRTC %d using output %c\n",
> -   drm_get_connector_name(
> -   &nouveau_encoder_connector_get(nv_encoder)->base),
> +   (&nouveau_encoder_connector_get(nv_encoder)->base)->name,
> nv_crtc->index, '@' + ffs(nv_encoder->dcb->or));
>  }
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index d07ce028af51..6ecea9b2b15a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -265,14 +265,14 @@ nouveau_connector_detect(struct drm_connector 
> *connector, bool force)
> nv_connector->edid);
> if (!nv_connector->edid) {
> NV_ERROR(drm, "DDC responded, but no EDID for %s\n",
> -drm_get_connector_na