* [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: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
* 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
* 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
* ✓ 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
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.