All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
@ 2017-09-26 21:13 Rodrigo Vivi
  2017-09-26 21:21 ` Paulo Zanoni
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Rodrigo Vivi @ 2017-09-26 21:13 UTC (permalink / raw)
  To: intel-gfx; +Cc: Paulo Zanoni, Hans de Goede, Rodrigo Vivi, Daniel Vetter, Lyude

Let's stop this usage before it spreads so much.

1. This check is not part of usual searches happening when adding
new platform.
2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
and INTEL_GEN(dev_priv).

So let's please avoid yet another way.

Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with wms/plane attrs")
Cc: Lyude <cpaul@redhat.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Shashank Sharma <shashank.sharma@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 487b43ba3139..fba76f1bb6dd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->gamma_mode =
 		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
 
-	if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
+	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
 		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
 		bool clrspace_yuv = tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV;
 
-		if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >= 10) {
+		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) {
 			bool blend_mode_420 = tmp &
 					      PIPEMISC_YUV420_MODE_FULL_BLEND;
 
@@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
 	}
 
-	if (dev_priv->info.gen >= 9)
+	if (INTEL_GEN(dev_priv) >= 9)
 		dev_priv->display.update_crtcs = skl_update_crtcs;
 	else
 		dev_priv->display.update_crtcs = intel_update_crtcs;
-- 
2.13.5

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-26 21:13 [PATCH] drm/i915: Avoid using dev_priv->info.gen directly Rodrigo Vivi
@ 2017-09-26 21:21 ` Paulo Zanoni
  2017-09-26 21:47   ` Rodrigo Vivi
  2017-09-26 21:27 ` Matt Roper
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Paulo Zanoni @ 2017-09-26 21:21 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx; +Cc: Hans de Goede, Daniel Vetter, Lyude

Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
> Let's stop this usage before it spreads so much.
> 
> 1. This check is not part of usual searches happening when adding
> new platform.
> 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
> and INTEL_GEN(dev_priv).
> 
> So let's please avoid yet another way.
> 
> Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
> Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
> wms/plane attrs")

Not sure if the Fixes tags are appropriate since this is not a bug fix.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> Cc: Lyude <cpaul@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 487b43ba3139..fba76f1bb6dd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct
> intel_crtc *crtc,
>  	pipe_config->gamma_mode =
>  		I915_READ(GAMMA_MODE(crtc->pipe)) &
> GAMMA_MODE_MODE_MASK;
>  
> -	if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
> +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
>  		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
>  		bool clrspace_yuv = tmp &
> PIPEMISC_OUTPUT_COLORSPACE_YUV;
>  
> -		if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >=
> 10) {
> +		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
> >= 10) {
>  			bool blend_mode_420 = tmp &
>  					      PIPEMISC_YUV420_MODE_F
> ULL_BLEND;
>  
> @@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct
> drm_i915_private *dev_priv)
>  		dev_priv->display.fdi_link_train =
> hsw_fdi_link_train;
>  	}
>  
> -	if (dev_priv->info.gen >= 9)
> +	if (INTEL_GEN(dev_priv) >= 9)
>  		dev_priv->display.update_crtcs = skl_update_crtcs;
>  	else
>  		dev_priv->display.update_crtcs = intel_update_crtcs;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-26 21:13 [PATCH] drm/i915: Avoid using dev_priv->info.gen directly Rodrigo Vivi
  2017-09-26 21:21 ` Paulo Zanoni
@ 2017-09-26 21:27 ` Matt Roper
  2017-09-26 21:35 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-09-27  8:07 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Matt Roper @ 2017-09-26 21:27 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: Paulo Zanoni, intel-gfx, Hans de Goede, Daniel Vetter, Lyude

On Tue, Sep 26, 2017 at 02:13:46PM -0700, Rodrigo Vivi wrote:
> Let's stop this usage before it spreads so much.
> 
> 1. This check is not part of usual searches happening when adding
> new platform.
> 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
> and INTEL_GEN(dev_priv).
> 
> So let's please avoid yet another way.
> 
> Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
> Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with wms/plane attrs")
> Cc: Lyude <cpaul@redhat.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Cc: Shashank Sharma <shashank.sharma@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Maybe we could make dim use coccinelle to flag these kinds of patterns
in the future to prevent them from sneaking back into the code?


Matt

> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 487b43ba3139..fba76f1bb6dd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	pipe_config->gamma_mode =
>  		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
>  
> -	if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
> +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
>  		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
>  		bool clrspace_yuv = tmp & PIPEMISC_OUTPUT_COLORSPACE_YUV;
>  
> -		if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >= 10) {
> +		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10) {
>  			bool blend_mode_420 = tmp &
>  					      PIPEMISC_YUV420_MODE_FULL_BLEND;
>  
> @@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  		dev_priv->display.fdi_link_train = hsw_fdi_link_train;
>  	}
>  
> -	if (dev_priv->info.gen >= 9)
> +	if (INTEL_GEN(dev_priv) >= 9)
>  		dev_priv->display.update_crtcs = skl_update_crtcs;
>  	else
>  		dev_priv->display.update_crtcs = intel_update_crtcs;
> -- 
> 2.13.5
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-26 21:13 [PATCH] drm/i915: Avoid using dev_priv->info.gen directly Rodrigo Vivi
  2017-09-26 21:21 ` Paulo Zanoni
  2017-09-26 21:27 ` Matt Roper
@ 2017-09-26 21:35 ` Patchwork
  2017-09-27  8:07 ` ✓ Fi.CI.IGT: " Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-09-26 21:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Avoid using dev_priv->info.gen directly.
URL   : https://patchwork.freedesktop.org/series/30926/
State : success

== Summary ==

Series 30926v1 drm/i915: Avoid using dev_priv->info.gen directly.
https://patchwork.freedesktop.org/api/1.0/series/30926/revisions/1/mbox/

Test chamelium:
        Subgroup dp-edid-read:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102672
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-glk-1) fdo#102777 +1

fdo#102672 https://bugs.freedesktop.org/show_bug.cgi?id=102672
fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#102294 https://bugs.freedesktop.org/show_bug.cgi?id=102294
fdo#102777 https://bugs.freedesktop.org/show_bug.cgi?id=102777

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:440s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:466s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:422s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:517s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:278s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:495s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:496s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:497s
fi-cfl-s         total:289  pass:223  dwarn:34  dfail:0   fail:0   skip:32  time:539s
fi-cnl-y         total:289  pass:258  dwarn:0   dfail:0   fail:4   skip:27  time:639s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:418s
fi-glk-1         total:289  pass:259  dwarn:1   dfail:0   fail:0   skip:29  time:564s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:423s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:405s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:433s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:491s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:460s
fi-kbl-7500u     total:289  pass:263  dwarn:1   dfail:0   fail:1   skip:24  time:469s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:577s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:589s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:549s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:748s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:487s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:475s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:575s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:414s

13d4fadfbe072f2952fb0fda72f2176434b609f5 drm-tip: 2017y-09m-26d-20h-03m-18s UTC integration manifest
a457f6bb37eb drm/i915: Avoid using dev_priv->info.gen directly.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5826/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-26 21:21 ` Paulo Zanoni
@ 2017-09-26 21:47   ` Rodrigo Vivi
  2017-09-27  7:03     ` Jani Nikula
  0 siblings, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2017-09-26 21:47 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, Hans de Goede, Daniel Vetter, Lyude

On Tue, Sep 26, 2017 at 09:21:43PM +0000, Paulo Zanoni wrote:
> Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
> > Let's stop this usage before it spreads so much.
> > 
> > 1. This check is not part of usual searches happening when adding
> > new platform.
> > 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
> > and INTEL_GEN(dev_priv).
> > 
> > So let's please avoid yet another way.
> > 
> > Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
> > Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
> > wms/plane attrs")
> 
> Not sure if the Fixes tags are appropriate since this is not a bug fix.

I wondered that... but since "dim fixes" provided me that tag along with the
list of people I should cc I decided to include here. I thought it
wouldn't hurt and also maybe good to propagate that to everywhere possible so
we don't recieve more code based on that usage.

But I won't merge today to give time to get Jani's view on that.

> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> > Cc: Lyude <cpaul@redhat.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> > Cc: Matt Roper <matthew.d.roper@intel.com>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Cc: Shashank Sharma <shashank.sharma@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > b/drivers/gpu/drm/i915/intel_display.c
> > index 487b43ba3139..fba76f1bb6dd 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct
> > intel_crtc *crtc,
> >  	pipe_config->gamma_mode =
> >  		I915_READ(GAMMA_MODE(crtc->pipe)) &
> > GAMMA_MODE_MODE_MASK;
> >  
> > -	if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
> > +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
> >  		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
> >  		bool clrspace_yuv = tmp &
> > PIPEMISC_OUTPUT_COLORSPACE_YUV;
> >  
> > -		if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >=
> > 10) {
> > +		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
> > >= 10) {
> >  			bool blend_mode_420 = tmp &
> >  					      PIPEMISC_YUV420_MODE_F
> > ULL_BLEND;
> >  
> > @@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct
> > drm_i915_private *dev_priv)
> >  		dev_priv->display.fdi_link_train =
> > hsw_fdi_link_train;
> >  	}
> >  
> > -	if (dev_priv->info.gen >= 9)
> > +	if (INTEL_GEN(dev_priv) >= 9)
> >  		dev_priv->display.update_crtcs = skl_update_crtcs;
> >  	else
> >  		dev_priv->display.update_crtcs = intel_update_crtcs;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-26 21:47   ` Rodrigo Vivi
@ 2017-09-27  7:03     ` Jani Nikula
  2017-09-27 19:32       ` Rodrigo Vivi
  0 siblings, 1 reply; 10+ messages in thread
From: Jani Nikula @ 2017-09-27  7:03 UTC (permalink / raw)
  To: Rodrigo Vivi, Paulo Zanoni; +Cc: intel-gfx, Hans de Goede, Daniel Vetter, Lyude

On Tue, 26 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Tue, Sep 26, 2017 at 09:21:43PM +0000, Paulo Zanoni wrote:
>> Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
>> > Let's stop this usage before it spreads so much.
>> > 
>> > 1. This check is not part of usual searches happening when adding
>> > new platform.
>> > 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
>> > and INTEL_GEN(dev_priv).
>> > 
>> > So let's please avoid yet another way.
>> > 
>> > Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
>> > Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
>> > wms/plane attrs")
>> 
>> Not sure if the Fixes tags are appropriate since this is not a bug fix.
>
> I wondered that... but since "dim fixes" provided me that tag along with the
> list of people I should cc I decided to include here. I thought it
> wouldn't hurt and also maybe good to propagate that to everywhere possible so
> we don't recieve more code based on that usage.
>
> But I won't merge today to give time to get Jani's view on that.

Please only use Fixes: for functional fixes that need to be
backported. Like, nobody's going to be happier running a kernel they
know uses INTEL_GEN() consistently.

BR,
Jani.


>
>> 
>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> 
>> > Cc: Lyude <cpaul@redhat.com>
>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > Cc: Daniel Vetter <daniel.vetter@intel.com>
>> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> > Cc: Hans de Goede <hdegoede@redhat.com>
>> > Cc: Matt Roper <matthew.d.roper@intel.com>
>> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>> > Cc: Imre Deak <imre.deak@intel.com>
>> > Cc: Shashank Sharma <shashank.sharma@intel.com>
>> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>> > 
>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>> > b/drivers/gpu/drm/i915/intel_display.c
>> > index 487b43ba3139..fba76f1bb6dd 100644
>> > --- a/drivers/gpu/drm/i915/intel_display.c
>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>> > @@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct
>> > intel_crtc *crtc,
>> >  	pipe_config->gamma_mode =
>> >  		I915_READ(GAMMA_MODE(crtc->pipe)) &
>> > GAMMA_MODE_MODE_MASK;
>> >  
>> > -	if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
>> > +	if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
>> >  		u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
>> >  		bool clrspace_yuv = tmp &
>> > PIPEMISC_OUTPUT_COLORSPACE_YUV;
>> >  
>> > -		if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >=
>> > 10) {
>> > +		if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
>> > >= 10) {
>> >  			bool blend_mode_420 = tmp &
>> >  					      PIPEMISC_YUV420_MODE_F
>> > ULL_BLEND;
>> >  
>> > @@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct
>> > drm_i915_private *dev_priv)
>> >  		dev_priv->display.fdi_link_train =
>> > hsw_fdi_link_train;
>> >  	}
>> >  
>> > -	if (dev_priv->info.gen >= 9)
>> > +	if (INTEL_GEN(dev_priv) >= 9)
>> >  		dev_priv->display.update_crtcs = skl_update_crtcs;
>> >  	else
>> >  		dev_priv->display.update_crtcs = intel_update_crtcs;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-26 21:13 [PATCH] drm/i915: Avoid using dev_priv->info.gen directly Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2017-09-26 21:35 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-09-27  8:07 ` Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-09-27  8:07 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Avoid using dev_priv->info.gen directly.
URL   : https://patchwork.freedesktop.org/series/30926/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252

fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-hsw        total:2429 pass:1336 dwarn:1   dfail:0   fail:9   skip:1083 time:9949s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5826/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-27  7:03     ` Jani Nikula
@ 2017-09-27 19:32       ` Rodrigo Vivi
  2017-09-29  9:05         ` Joonas Lahtinen
  0 siblings, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2017-09-27 19:32 UTC (permalink / raw)
  To: Jani Nikula
  Cc: Paulo Zanoni, intel-gfx, Hans de Goede, Rodrigo Vivi,
	Daniel Vetter, Lyude

On Wed, Sep 27, 2017 at 12:03 AM, Jani Nikula
<jani.nikula@linux.intel.com> wrote:
> On Tue, 26 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> On Tue, Sep 26, 2017 at 09:21:43PM +0000, Paulo Zanoni wrote:
>>> Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
>>> > Let's stop this usage before it spreads so much.
>>> >
>>> > 1. This check is not part of usual searches happening when adding
>>> > new platform.
>>> > 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
>>> > and INTEL_GEN(dev_priv).
>>> >
>>> > So let's please avoid yet another way.
>>> >
>>> > Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
>>> > Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
>>> > wms/plane attrs")
>>>
>>> Not sure if the Fixes tags are appropriate since this is not a bug fix.
>>
>> I wondered that... but since "dim fixes" provided me that tag along with the
>> list of people I should cc I decided to include here. I thought it
>> wouldn't hurt and also maybe good to propagate that to everywhere possible so
>> we don't recieve more code based on that usage.
>>
>> But I won't merge today to give time to get Jani's view on that.
>
> Please only use Fixes: for functional fixes that need to be
> backported. Like, nobody's going to be happier running a kernel they
> know uses INTEL_GEN() consistently.

Makes sense.
Merged to dinq without the "Fixes:" tags.
Thanks for all comments and reviews.

>
> BR,
> Jani.
>
>
>>
>>>
>>> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>>
>>> > Cc: Lyude <cpaul@redhat.com>
>>> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> > Cc: Daniel Vetter <daniel.vetter@intel.com>
>>> > Cc: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>>> > Cc: Hans de Goede <hdegoede@redhat.com>
>>> > Cc: Matt Roper <matthew.d.roper@intel.com>
>>> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
>>> > Cc: Imre Deak <imre.deak@intel.com>
>>> > Cc: Shashank Sharma <shashank.sharma@intel.com>
>>> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
>>> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/intel_display.c | 6 +++---
>>> >  1 file changed, 3 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/intel_display.c
>>> > b/drivers/gpu/drm/i915/intel_display.c
>>> > index 487b43ba3139..fba76f1bb6dd 100644
>>> > --- a/drivers/gpu/drm/i915/intel_display.c
>>> > +++ b/drivers/gpu/drm/i915/intel_display.c
>>> > @@ -9310,11 +9310,11 @@ static bool haswell_get_pipe_config(struct
>>> > intel_crtc *crtc,
>>> >    pipe_config->gamma_mode =
>>> >            I915_READ(GAMMA_MODE(crtc->pipe)) &
>>> > GAMMA_MODE_MODE_MASK;
>>> >
>>> > -  if (IS_BROADWELL(dev_priv) || dev_priv->info.gen >= 9) {
>>> > +  if (IS_BROADWELL(dev_priv) || INTEL_GEN(dev_priv) >= 9) {
>>> >            u32 tmp = I915_READ(PIPEMISC(crtc->pipe));
>>> >            bool clrspace_yuv = tmp &
>>> > PIPEMISC_OUTPUT_COLORSPACE_YUV;
>>> >
>>> > -          if (IS_GEMINILAKE(dev_priv) || dev_priv->info.gen >=
>>> > 10) {
>>> > +          if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv)
>>> > >= 10) {
>>> >                    bool blend_mode_420 = tmp &
>>> >                                          PIPEMISC_YUV420_MODE_F
>>> > ULL_BLEND;
>>> >
>>> > @@ -14217,7 +14217,7 @@ void intel_init_display_hooks(struct
>>> > drm_i915_private *dev_priv)
>>> >            dev_priv->display.fdi_link_train =
>>> > hsw_fdi_link_train;
>>> >    }
>>> >
>>> > -  if (dev_priv->info.gen >= 9)
>>> > +  if (INTEL_GEN(dev_priv) >= 9)
>>> >            dev_priv->display.update_crtcs = skl_update_crtcs;
>>> >    else
>>> >            dev_priv->display.update_crtcs = intel_update_crtcs;
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-27 19:32       ` Rodrigo Vivi
@ 2017-09-29  9:05         ` Joonas Lahtinen
  2017-10-02  8:33           ` Jani Nikula
  0 siblings, 1 reply; 10+ messages in thread
From: Joonas Lahtinen @ 2017-09-29  9:05 UTC (permalink / raw)
  To: Rodrigo Vivi, Jani Nikula, Chris Wilson
  Cc: Paulo Zanoni, intel-gfx, Hans de Goede, Rodrigo Vivi,
	Daniel Vetter, Lyude

On Wed, 2017-09-27 at 12:32 -0700, Rodrigo Vivi wrote:
> On Wed, Sep 27, 2017 at 12:03 AM, Jani Nikula
> <jani.nikula@linux.intel.com> wrote:
> > On Tue, 26 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > > On Tue, Sep 26, 2017 at 09:21:43PM +0000, Paulo Zanoni wrote:
> > > > Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
> > > > > Let's stop this usage before it spreads so much.
> > > > > 
> > > > > 1. This check is not part of usual searches happening when adding
> > > > > new platform.
> > > > > 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
> > > > > and INTEL_GEN(dev_priv).
> > > > > 
> > > > > So let's please avoid yet another way.
> > > > > 
> > > > > Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
> > > > > Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
> > > > > wms/plane attrs")
> > > > 
> > > > Not sure if the Fixes tags are appropriate since this is not a bug fix.
> > > 
> > > I wondered that... but since "dim fixes" provided me that tag along with the
> > > list of people I should cc I decided to include here. I thought it
> > > wouldn't hurt and also maybe good to propagate that to everywhere possible so
> > > we don't recieve more code based on that usage.
> > > 
> > > But I won't merge today to give time to get Jani's view on that.
> > 
> > Please only use Fixes: for functional fixes that need to be
> > backported. Like, nobody's going to be happier running a kernel they
> > know uses INTEL_GEN() consistently.
> 
> Makes sense.
> Merged to dinq without the "Fixes:" tags.
> Thanks for all comments and reviews.

We discussed this with Chris too, I ended up suggesting there could be
something along; "Backport: none" or "Backport: v4.0+", instead of the
horrible #v4.0 that is causing pain every now and then, by getting
mixed to the To: fields in a wrong way.

Any thoughts on that?

Fixes: is an interesting metric in other sense too, or then just decide
to use X-Fixes: when we don't want it to end up backported.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915: Avoid using dev_priv->info.gen directly.
  2017-09-29  9:05         ` Joonas Lahtinen
@ 2017-10-02  8:33           ` Jani Nikula
  0 siblings, 0 replies; 10+ messages in thread
From: Jani Nikula @ 2017-10-02  8:33 UTC (permalink / raw)
  To: Joonas Lahtinen, Rodrigo Vivi, Chris Wilson
  Cc: Paulo Zanoni, intel-gfx, Hans de Goede, Rodrigo Vivi,
	Daniel Vetter, Lyude

On Fri, 29 Sep 2017, Joonas Lahtinen <joonas.lahtinen@linux.intel.com> wrote:
> On Wed, 2017-09-27 at 12:32 -0700, Rodrigo Vivi wrote:
>> On Wed, Sep 27, 2017 at 12:03 AM, Jani Nikula
>> <jani.nikula@linux.intel.com> wrote:
>> > On Tue, 26 Sep 2017, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
>> > > On Tue, Sep 26, 2017 at 09:21:43PM +0000, Paulo Zanoni wrote:
>> > > > Em Ter, 2017-09-26 às 14:13 -0700, Rodrigo Vivi escreveu:
>> > > > > Let's stop this usage before it spreads so much.
>> > > > > 
>> > > > > 1. This check is not part of usual searches happening when adding
>> > > > > new platform.
>> > > > > 2. There is already a duplication here with INTEL_INFO(dev_priv)->gen
>> > > > > and INTEL_GEN(dev_priv).
>> > > > > 
>> > > > > So let's please avoid yet another way.
>> > > > > 
>> > > > > Fixes: b22ca995ba1c ("drm/i915: prepare pipe for YCBCR420 output")
>> > > > > Fixes: 27082493e9c6 ("drm/i915/skl: Update DDB values atomically with
>> > > > > wms/plane attrs")
>> > > > 
>> > > > Not sure if the Fixes tags are appropriate since this is not a bug fix.
>> > > 
>> > > I wondered that... but since "dim fixes" provided me that tag along with the
>> > > list of people I should cc I decided to include here. I thought it
>> > > wouldn't hurt and also maybe good to propagate that to everywhere possible so
>> > > we don't recieve more code based on that usage.
>> > > 
>> > > But I won't merge today to give time to get Jani's view on that.
>> > 
>> > Please only use Fixes: for functional fixes that need to be
>> > backported. Like, nobody's going to be happier running a kernel they
>> > know uses INTEL_GEN() consistently.
>> 
>> Makes sense.
>> Merged to dinq without the "Fixes:" tags.
>> Thanks for all comments and reviews.
>
> We discussed this with Chris too, I ended up suggesting there could be
> something along; "Backport: none" or "Backport: v4.0+", instead of the
> horrible #v4.0 that is causing pain every now and then, by getting
> mixed to the To: fields in a wrong way.
>
> Any thoughts on that?

The "# v4.0" notation is described in stable-kernel-rules.rst, and if
there are problems with it, they will get fixed. Git screwed it up, and
AFAIK it has since been fixed. I think stick with what the stable team
wants.

> Fixes: is an interesting metric in other sense too, or then just decide
> to use X-Fixes: when we don't want it to end up backported.

IMO Fixes: should only be used for actual functional fixes, not cosmetic
stuff. We don't need to track cosmetic fixes, nobody wants to backport
them anywhere. Our tooling heavily relies on Fixes: to select fixes
backports, and we'll just look silly sending "comment typo fix" style
patches Linus' way at, say, -rc6 when stuff is supposed to abide by
stable-kernel-rules.rst.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-02  8:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-26 21:13 [PATCH] drm/i915: Avoid using dev_priv->info.gen directly Rodrigo Vivi
2017-09-26 21:21 ` Paulo Zanoni
2017-09-26 21:47   ` Rodrigo Vivi
2017-09-27  7:03     ` Jani Nikula
2017-09-27 19:32       ` Rodrigo Vivi
2017-09-29  9:05         ` Joonas Lahtinen
2017-10-02  8:33           ` Jani Nikula
2017-09-26 21:27 ` Matt Roper
2017-09-26 21:35 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-09-27  8:07 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.