date:20180413

Re: [Intel-gfx] [PATCH] drm/i915: Refine the has_audio assignment

2018-04-13 Thread Chris Wilson

Quoting Yang (2018-04-13 05:06:45)
> From: Yang Shi 
> 
> Refine the has_audio assignment for dp and hdmi.

s/Refine/Ignore the user override for/

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

Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Remove GUC_CTL_DEVICE_INFO parameter

2018-04-13 Thread Piorkowski, Piotr

On Tue, 2018-03-06 at 10:07 +0530, Sagar Arun Kamble wrote:
> 
> On 3/5/2018 6:43 PM, Piotr Piórkowski wrote:
> > It looks that GuC does not actively use GUC_CTL_DEVICE_INFO
> > parameter
> > where we are passing GT type and Core family values.
> > Lets stop setup this parameter and remove related definitions.
> 
> Minor change to sentence above: Let's stop/remove setup of this 
> parameter ...
> > 
> > v2: (this time without squashed HAX)
> >- New title and description
> >- Remove also GUC_CORE_FAMILY_* definitions (Michel)
> > 
> > Signed-off-by: Piotr Piórkowski 
> > Cc: Sagar Arun Kamble 
> > Cc: Michał Winiarski 
> > Cc: John A Spotswood 
> > Cc: Michal Wajdeczko 
> > Cc: Chris Wilson 
> > Cc: Michel Thierry 
> 
> With Michel's suggestion and then corresponding subject update patch 
> looks good to me.
> Reviewed-by: Sagar Arun Kamble 
> > ---
> >   drivers/gpu/drm/i915/intel_guc.c  | 24 --
> > --
> >   drivers/gpu/drm/i915/intel_guc_fwif.h |  7 ---
> >   2 files changed, 31 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_guc.c
> > b/drivers/gpu/drm/i915/intel_guc.c
> > index ff08ea0ebf49..efc413137a89 100644
> > --- a/drivers/gpu/drm/i915/intel_guc.c
> > +++ b/drivers/gpu/drm/i915/intel_guc.c
> > @@ -200,26 +200,6 @@ void intel_guc_fini(struct intel_guc *guc)
> > guc_shared_data_destroy(guc);
> >   }
> >   
> > -static u32 get_gt_type(struct drm_i915_private *dev_priv)
> > -{
> > -   /* XXX: GT type based on PCI device ID? field seems unused
> > by fw */
> > -   return 0;
> > -}
> > -
> > -static u32 get_core_family(struct drm_i915_private *dev_priv)
> > -{
> > -   u32 gen = INTEL_GEN(dev_priv);
> > -
> > -   switch (gen) {
> > -   case 9:
> > -   return GUC_CORE_FAMILY_GEN9;
> > -
> > -   default:
> > -   MISSING_CASE(gen);
> > -   return GUC_CORE_FAMILY_UNKNOWN;
> > -   }
> > -}
> > -
> >   static u32 get_log_verbosity_flags(void)
> >   {
> > if (i915_modparams.guc_log_level > 0) {
> > @@ -246,10 +226,6 @@ void intel_guc_init_params(struct intel_guc
> > *guc)
> >   
> > memset(params, 0, sizeof(params));
> >   
> > -   params[GUC_CTL_DEVICE_INFO] |=
> > -   (get_gt_type(dev_priv) << GUC_CTL_GT_TYPE_SHIFT) |
> > -   (get_core_family(dev_priv) <<
> > GUC_CTL_CORE_FAMILY_SHIFT);
> > -
> > /*
> >  * GuC ARAT increment is 10 ns. GuC default scheduler
> > quantum is one
> >  * second. This ARAR is calculated by:
> > diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > index 6a10aa6f04d3..5131e67e663f 100644
> > --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> > +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> > @@ -23,9 +23,6 @@
> >   #ifndef _INTEL_GUC_FWIF_H
> >   #define _INTEL_GUC_FWIF_H
> >   
> > -#define GUC_CORE_FAMILY_GEN9   12
> > -#define GUC_CORE_FAMILY_UNKNOWN0x7fff
> > -
> >   #define GUC_CLIENT_PRIORITY_KMD_HIGH  0
> >   #define GUC_CLIENT_PRIORITY_HIGH  1
> >   #define GUC_CLIENT_PRIORITY_KMD_NORMAL2
> > @@ -81,10 +78,6 @@
> >   #define GUC_CTL_ARAT_HIGH 1
> >   #define GUC_CTL_ARAT_LOW  2
> >   
> > -#define GUC_CTL_DEVICE_INFO3
> > -#define   GUC_CTL_GT_TYPE_SHIFT0
> > -#define   GUC_CTL_CORE_FAMILY_SHIFT7
> > -
> >   #define GUC_CTL_LOG_PARAMS4
> >   #define   GUC_LOG_VALID   (1 << 0)
> >   #define   GUC_LOG_NOTIFY_ON_HALF_FULL (1 << 1)
> 
> 
What's next with this patch? Can we merge it ?

smime.p7s
Description: S/MIME cryptographic signature
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v4] drm/i915/guc: Remove GUC_CTL_DEVICE_INFO parameter

2018-04-13 Thread Piotr Piórkowski

It looks that GuC does not actively use GUC_CTL_DEVICE_INFO parameter
where we are passing GT type and Core family values.
Let's stop/remove setup of this parameter and remove related
definitions.

v2: (this time without squashed HAX)
  - New title and description
  - Remove also GUC_CORE_FAMILY_* definitions (Michel)
v3:
  - The removed define GUC_CTL_DEVICE_INFO has been restored (Michel)
  - Updated description (Sagar)
v4: rebase

Signed-off-by: Piotr Piórkowski 
Cc: Sagar Arun Kamble 
Cc: Michał Winiarski 
Cc: John A Spotswood 
Cc: Michal Wajdeczko 
Cc: Chris Wilson 
Cc: Michel Thierry 
Acked-by: Michel Thierry 
Reviewed-by: Sagar Arun Kamble 
---
 drivers/gpu/drm/i915/intel_guc.c  | 24 
 drivers/gpu/drm/i915/intel_guc_fwif.h |  5 -
 2 files changed, 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index a00a59a7d9ec..116f4ccf1bbd 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -203,26 +203,6 @@ void intel_guc_fini(struct intel_guc *guc)
guc_shared_data_destroy(guc);
 }
 
-static u32 get_gt_type(struct drm_i915_private *dev_priv)
-{
-   /* XXX: GT type based on PCI device ID? field seems unused by fw */
-   return 0;
-}
-
-static u32 get_core_family(struct drm_i915_private *dev_priv)
-{
-   u32 gen = INTEL_GEN(dev_priv);
-
-   switch (gen) {
-   case 9:
-   return GUC_CORE_FAMILY_GEN9;
-
-   default:
-   MISSING_CASE(gen);
-   return GUC_CORE_FAMILY_UNKNOWN;
-   }
-}
-
 static u32 get_log_control_flags(void)
 {
u32 level = i915_modparams.guc_log_level;
@@ -255,10 +235,6 @@ void intel_guc_init_params(struct intel_guc *guc)
 
memset(params, 0, sizeof(params));
 
-   params[GUC_CTL_DEVICE_INFO] |=
-   (get_gt_type(dev_priv) << GUC_CTL_GT_TYPE_SHIFT) |
-   (get_core_family(dev_priv) << GUC_CTL_CORE_FAMILY_SHIFT);
-
/*
 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
 * second. This ARAR is calculated by:
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/intel_guc_fwif.h
index d73673f5d30c..0867ba76d445 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -23,9 +23,6 @@
 #ifndef _INTEL_GUC_FWIF_H
 #define _INTEL_GUC_FWIF_H
 
-#define GUC_CORE_FAMILY_GEN9   12
-#define GUC_CORE_FAMILY_UNKNOWN0x7fff
-
 #define GUC_CLIENT_PRIORITY_KMD_HIGH   0
 #define GUC_CLIENT_PRIORITY_HIGH   1
 #define GUC_CLIENT_PRIORITY_KMD_NORMAL 2
@@ -82,8 +79,6 @@
 #define GUC_CTL_ARAT_LOW   2
 
 #define GUC_CTL_DEVICE_INFO3
-#define   GUC_CTL_GT_TYPE_SHIFT0
-#define   GUC_CTL_CORE_FAMILY_SHIFT7
 
 #define GUC_CTL_LOG_PARAMS 4
 #define   GUC_LOG_VALID(1 << 0)
-- 
2.14.3

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

Re: [Intel-gfx] [PATCH] drm/i915: Block enabling FBC until flips have been completed

2018-04-13 Thread Maarten Lankhorst

Op 12-04-18 om 21:41 schreef Souza, Jose:
> On Thu, 2018-04-12 at 18:07 +0200, Maarten Lankhorst wrote:
>> There is a small race window in which FBC can be enabled after
>> pre_plane_update is called, but before the page flip has been
>> queued or completed.
> I don't think there is such window, intel_fbc_deactivate() that is
> called from intel_fbc_pre_update() will set fbc->work.scheduled =
> false; so the FBC will not be enabled in intel_fbc_work_fn()
Yeah, intel_fbc_pre_update deactivates it, but intel_fbc_flush() can re-enable 
it. :)
>> Signed-off-by: Maarten Lankhorst 
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>>  drivers/gpu/drm/i915/intel_fbc.c | 35 +++---
>> -
>>  2 files changed, 12 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index a0b8db3db141..2e2f24c2db9e 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -492,6 +492,7 @@ struct intel_fbc {
>>  
>>  bool enabled;
>>  bool active;
>> +bool flip_pending;
>>  
>>  bool underrun_detected;
>>  struct work_struct underrun_work;
>> diff --git a/drivers/gpu/drm/i915/intel_fbc.c
>> b/drivers/gpu/drm/i915/intel_fbc.c
>> index b431b6733cc1..4770dd7dad5c 100644
>> --- a/drivers/gpu/drm/i915/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/intel_fbc.c
>> @@ -924,13 +924,6 @@ static void intel_fbc_get_reg_params(struct
>> intel_crtc *crtc,
>>  32 * fbc->threshold) 
>> * 8;
>>  }
>>  
>> -static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params
>> *params1,
>> -   struct intel_fbc_reg_params
>> *params2)
>> -{
>> -/* We can use this since intel_fbc_get_reg_params() does a
>> memset. */
>> -return memcmp(params1, params2, sizeof(*params1)) == 0;
>> -}
>> -
>>  void intel_fbc_pre_update(struct intel_crtc *crtc,
>>struct intel_crtc_state *crtc_state,
>>struct intel_plane_state *plane_state)
>> @@ -952,6 +945,7 @@ void intel_fbc_pre_update(struct intel_crtc
>> *crtc,
>>  if (!fbc->enabled || fbc->crtc != crtc)
>>  goto unlock;
>>  
>> +fbc->flip_pending = true;
> Also this is not a good name, other actions can cause this function to
> be executed other than a flip.
Well, for FBC purposes it's a flip, but I am also ok with renaming it to 
update_pending? :)
>>  intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
>>  
>>  deactivate:
>> @@ -988,13 +982,15 @@ static void __intel_fbc_post_update(struct
>> intel_crtc *crtc)
>>  {
>>  struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>  struct intel_fbc *fbc = _priv->fbc;
>> -struct intel_fbc_reg_params old_params;
>>  
>>  WARN_ON(!mutex_is_locked(>lock));
>>  
>>  if (!fbc->enabled || fbc->crtc != crtc)
>>  return;
>>  
>> +fbc->flip_pending = false;
>> +WARN_ON(fbc->active);
>> +
>>  if (!i915_modparams.enable_fbc) {
>>  intel_fbc_deactivate(dev_priv, "disabled at runtime
>> per module param");
>>  __intel_fbc_disable(dev_priv);
>> @@ -1002,25 +998,16 @@ static void __intel_fbc_post_update(struct
>> intel_crtc *crtc)
>>  return;
>>  }
>>  
>> -if (!intel_fbc_can_activate(crtc)) {
>> -WARN_ON(fbc->active);
>> +if (!intel_fbc_can_activate(crtc))
>>  return;
>> -}
>>  
>> -old_params = fbc->params;
>>  intel_fbc_get_reg_params(crtc, >params);
>>  
>> -/* If the scanout has not changed, don't modify the FBC
>> settings.
>> - * Note that we make the fundamental assumption that the fb-
>>> obj
>> - * cannot be unpinned (and have its GTT offset and fence
>> revoked)
>> - * without first being decoupled from the scanout and FBC
>> disabled.
>> - */
>> -if (fbc->active &&
>> -intel_fbc_reg_params_equal(_params, >params))
>> -return;
>> -
>> -intel_fbc_deactivate(dev_priv, "FBC enabled (active or
>> scheduled)");
>> -intel_fbc_schedule_activation(crtc);
>> +if (!fbc->busy_bits) {
> I guess this 'if' the line that is fixing the issue.

I think that's not necessarily the case for these tests. I don't know if this 
fixes
the bug, as the dirtyfb is called after the atomic update completed. I just 
noted that
after pre_plane_update, you could sneak in a dirtyfb and then fbc could be 
activated
too early.

That's the hole I've been trying to close. But I closed it the other way around 
too
just in case. :) 

>> +intel_fbc_deactivate(dev_priv, "FBC enabled (active
>> or scheduled)");
>> +intel_fbc_schedule_activation(crtc);
>> +} else
>> +intel_fbc_deactivate(dev_priv, "frontbuffer write");
>>  }
>>  
>>  void intel_fbc_post_update(struct intel_crtc *crtc)
>>

Re: [Intel-gfx] [PATCH v1 5/6] drm/i915: Do not do fb src adjustments for NV12

2018-04-13 Thread Srinivas, Vidya



> -Original Message-
> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
> Sent: Thursday, April 12, 2018 4:41 PM
> To: Srinivas, Vidya ; Intel Graphics Development
> ; Ville Syrjälä
> 
> Cc: Kamath, Sunil ; Saarinen, Jani
> 
> Subject: Re: [PATCH v1 5/6] drm/i915: Do not do fb src adjustments for
> NV12
> 
> Op 12-04-18 om 12:07 schreef Srinivas, Vidya:
> >
> >> -Original Message-
> >> From: Maarten Lankhorst [mailto:maarten.lankho...@linux.intel.com]
> >> Sent: Wednesday, April 11, 2018 4:08 PM
> >> To: Srinivas, Vidya ; intel-gfx-
> >> try...@lists.freedesktop.org
> >> Cc: Kamath, Sunil ; Saarinen, Jani
> >> 
> >> Subject: Re: [PATCH v1 5/6] drm/i915: Do not do fb src adjustments
> >> for
> >> NV12
> >>
> >> Op 11-04-18 om 11:09 schreef Vidya Srinivas:
> >>> We skip src trunction/adjustments for
> >>> NV12 case and handle the sizes directly.
> >>> Without this, pipe fifo underruns are seen on APL/KBL.
> >>>
> >>> Credits-to: Maarten Lankhorst 
> >>> Signed-off-by: Vidya Srinivas 
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_display.c | 88
> >>> ++--
> >>>  drivers/gpu/drm/i915/intel_sprite.c  | 10 +++-
> >>>  2 files changed, 92 insertions(+), 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>> b/drivers/gpu/drm/i915/intel_display.c
> >>> index ebb3f8e..e4cf7a6 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -12951,6 +12951,86 @@ skl_max_scale(struct intel_crtc
> >>> *intel_crtc, }
> >>>
> >>>  static int
> >>> +intel_primary_plane_state(struct drm_plane_state *plane_state,
> >>> +   const struct drm_crtc_state *crtc_state,
> >>> +   int min_scale, int max_scale,
> >>> +   bool can_position, bool can_update_disabled) {
> >>> + struct drm_framebuffer *fb = plane_state->fb;
> >>> + struct drm_rect *src = _state->src;
> >>> + struct drm_rect *dst = _state->dst;
> >>> + unsigned int rotation = plane_state->rotation;
> >>> + struct drm_rect clip = {};
> >>> + int hscale, vscale;
> >>> +
> >>> + WARN_ON(plane_state->crtc && plane_state->crtc != crtc_state-
> >>> crtc);
> >>> +
> >>> + *src = drm_plane_state_src(plane_state);
> >>> + *dst = drm_plane_state_dest(plane_state);
> >>> +
> >>> + if (!fb) {
> >>> + plane_state->visible = false;
> >>> + return 0;
> >>> + }
> >>> +
> >>> + /* crtc should only be NULL when disabling (i.e., !fb) */
> >>> + if (WARN_ON(!plane_state->crtc)) {
> >>> + plane_state->visible = false;
> >>> + return 0;
> >>> + }
> >>> +
> >>> + if (!crtc_state->enable && !can_update_disabled) {
> >>> + DRM_DEBUG_KMS("Cannot update plane of a disabled
> >> CRTC.\n");
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + drm_rect_rotate(src, fb->width << 16, fb->height << 16, rotation);
> >>> +
> >>> + /* Check scaling */
> >>> + hscale = drm_rect_calc_hscale(src, dst, min_scale, max_scale);
> >>> + vscale = drm_rect_calc_vscale(src, dst, min_scale, max_scale);
> >>> + if (hscale < 0 || vscale < 0) {
> >>> + DRM_DEBUG_KMS("Invalid scaling of plane\n");
> >>> + drm_rect_debug_print("src: ", _state->src, true);
> >>> + drm_rect_debug_print("dst: ", _state->dst, false);
> >>> + return -ERANGE;
> >>> + }
> >>> +
> >>> + if (crtc_state->enable)
> >>> + drm_mode_get_hv_timing(_state->mode, ,
> >> );
> >>> +
> >>> + if (fb->format->format == DRM_FORMAT_NV12) {
> >>> + plane_state->visible = true;
> >>> + goto skip_clip;
> >>> + }
> >>> +
> >>> + plane_state->visible =
> >>> + drm_rect_clip_scaled(src, dst, , hscale, vscale);
> >> The real problem is that it needs to be a multiple of 4. I think the
> >> clipping here is harmless, we should just adjust intel_check_sprite_plane
> for >= SKL.
> >> This will make it so we don't have to duplicate its checks.
> >>
> >> For NV12 we still want to call clip_rect_scaled, but then adjust all
> >> coordinates by dividing by 4 on before the check, and multiplying
> >> with 4 after?
> >>
> > Thank you. Have made the changes in
> > https://patchwork.freedesktop.org/patch/216682/
> > With this, I did not see any underruns. For now, have a WA only when
> > we pass 16x16 Because, with that it further clips down and gets
> > rejected in skl_update_scaler as it is Less than 16. If we increase our
> buffer in igt, then this issue wont be there.
> > Please have a check. Initially, I tried the /4 before the adjustments and *4
> later, that wouldn’t work
> > We need to have the 16.16 values multiplier of 4. So, just put it towards
> the end of both plane checks.
> Well, this is annoying.
> 
>

Re: [Intel-gfx] [PATCH] drm/i915: Refine the has_audio assignment

Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Remove GUC_CTL_DEVICE_INFO parameter

[Intel-gfx] [PATCH v4] drm/i915/guc: Remove GUC_CTL_DEVICE_INFO parameter

Re: [Intel-gfx] [PATCH] drm/i915: Block enabling FBC until flips have been completed

Re: [Intel-gfx] [PATCH v1 5/6] drm/i915: Do not do fb src adjustments for NV12

< 1 2

101 - 105 of 105 matches

Site Navigation

Mail list logo

Footer information