| Message ID | 1479816065-29438-1-git-send-email-nabendu.bikash.maiti@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 22, 2016 at 05:31:05PM +0530, Nabendu Maiti wrote: Try to avoid confusing contractions (i.e. no no) and give us a verb in that sentence in that Subject line. > Number of scalers initialization is moved to runtime init from > intel_crtc_init for platform specific initialization. so that... Tell us why! > Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_device_info.c | 3 +++ > drivers/gpu/drm/i915/intel_display.c | 10 ++-------- > drivers/gpu/drm/i915/intel_drv.h | 4 ++-- > 5 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index b7f42c4..8349abe 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -3080,7 +3080,7 @@ static void intel_scaler_info(struct seq_file *m, struct intel_crtc *intel_crtc) > pipe_config->scaler_state.scaler_users, > pipe_config->scaler_state.scaler_id); > > - for (i = 0; i < SKL_NUM_SCALERS; i++) { > + for (i = 0; i < num_scalers; i++) { > struct intel_scaler *sc = > &pipe_config->scaler_state.scalers[i]; Already applied. > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index be67aee..6eed9c2 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -731,6 +731,7 @@ struct intel_device_info { > u16 device_id; > u8 num_pipes; > u8 num_sprites[I915_MAX_PIPES]; > + u8 num_scalers[I915_MAX_PIPES]; > u8 gen; > u16 gen_mask; > u8 ring_mask; /* Rings supported by the HW */ > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c > index 185e3bb..ef26fa8 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.c > +++ b/drivers/gpu/drm/i915/intel_device_info.c > @@ -282,6 +282,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) > info->num_sprites[PIPE_A] = 2; > info->num_sprites[PIPE_B] = 2; > info->num_sprites[PIPE_C] = 1; > + info->num_scalers[PIPE_A] = 2; > + info->num_scalers[PIPE_B] = 2; > + info->num_scalers[PIPE_C] = 1; > } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { > for_each_pipe(dev_priv, pipe) > info->num_sprites[pipe] = 2; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index bd2c99e..d2023c4 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15269,14 +15269,8 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) > crtc_state->base.crtc = &intel_crtc->base; > > /* initialize shared scalers */ > - if (INTEL_GEN(dev_priv) >= 9) { > - if (pipe == PIPE_C) > - intel_crtc->num_scalers = 1; > - else > - intel_crtc->num_scalers = SKL_NUM_SCALERS; > - > - skl_init_scalers(dev_priv, intel_crtc, crtc_state); > - } > + intel_crtc->num_scalers = dev_priv->info.num_scalers[pipe]; Double space after =. > + skl_init_scalers(dev_priv, intel_crtc, crtc_state); Now called for everybody, not just gen9+. Why? Is it safe? If it was safe it would not be called skl_*(). intel_crtc->num_scalers is now a candidate for including in init_scalers(). > primary = intel_primary_plane_create(dev_priv, pipe); > if (IS_ERR(primary)) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index cd132c2..3f89607 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -430,8 +430,8 @@ struct intel_scaler { > }; > > struct intel_crtc_scaler_state { > -#define SKL_NUM_SCALERS 2 > - struct intel_scaler scalers[SKL_NUM_SCALERS]; > +#define MAX_NUM_SCALERS 2 > + struct intel_scaler scalers[MAX_NUM_SCALERS]; Replaced the gen marker rather than the abreviation. So now we have what reads as NUM_NUM_SCALERS. -Chris
On 11/22/2016 9:05 PM, Chris Wilson wrote: > On Tue, Nov 22, 2016 at 05:31:05PM +0530, Nabendu Maiti wrote: > > Try to avoid confusing contractions (i.e. no no) and give us a verb in > that sentence in that Subject line. > >> Number of scalers initialization is moved to runtime init from >> intel_crtc_init for platform specific initialization. > > so that... Tell us why! Next patch set. > >> Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 2 +- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/intel_device_info.c | 3 +++ >> drivers/gpu/drm/i915/intel_display.c | 10 ++-------- >> drivers/gpu/drm/i915/intel_drv.h | 4 ++-- >> 5 files changed, 9 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index b7f42c4..8349abe 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -3080,7 +3080,7 @@ static void intel_scaler_info(struct seq_file *m, struct intel_crtc *intel_crtc) >> pipe_config->scaler_state.scaler_users, >> pipe_config->scaler_state.scaler_id); >> >> - for (i = 0; i < SKL_NUM_SCALERS; i++) { >> + for (i = 0; i < num_scalers; i++) { >> struct intel_scaler *sc = >> &pipe_config->scaler_state.scalers[i]; > > Already applied. > Removed from this patch. >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index be67aee..6eed9c2 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -731,6 +731,7 @@ struct intel_device_info { >> u16 device_id; >> u8 num_pipes; >> u8 num_sprites[I915_MAX_PIPES]; >> + u8 num_scalers[I915_MAX_PIPES]; >> u8 gen; >> u16 gen_mask; >> u8 ring_mask; /* Rings supported by the HW */ >> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c >> index 185e3bb..ef26fa8 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.c >> +++ b/drivers/gpu/drm/i915/intel_device_info.c >> @@ -282,6 +282,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) >> info->num_sprites[PIPE_A] = 2; >> info->num_sprites[PIPE_B] = 2; >> info->num_sprites[PIPE_C] = 1; >> + info->num_scalers[PIPE_A] = 2; >> + info->num_scalers[PIPE_B] = 2; >> + info->num_scalers[PIPE_C] = 1; >> } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { >> for_each_pipe(dev_priv, pipe) >> info->num_sprites[pipe] = 2; >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index bd2c99e..d2023c4 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -15269,14 +15269,8 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) >> crtc_state->base.crtc = &intel_crtc->base; >> >> /* initialize shared scalers */ >> - if (INTEL_GEN(dev_priv) >= 9) { >> - if (pipe == PIPE_C) >> - intel_crtc->num_scalers = 1; >> - else >> - intel_crtc->num_scalers = SKL_NUM_SCALERS; >> - >> - skl_init_scalers(dev_priv, intel_crtc, crtc_state); >> - } >> + intel_crtc->num_scalers = dev_priv->info.num_scalers[pipe]; > > Double space after =. > Done. >> + skl_init_scalers(dev_priv, intel_crtc, crtc_state); > > Now called for everybody, not just gen9+. Why? Is it safe? If it was > safe it would not be called skl_*(). intel_crtc->num_scalers is now a > candidate for including in init_scalers(). > Added check on numbers of scalers in each crtc in next version. If no scaler, return without initialization. Also can move skl_init_scalers to below plane initialization. Scaler is not utilized upto this point. In next patches will change the function names more generic. >> primary = intel_primary_plane_create(dev_priv, pipe); >> if (IS_ERR(primary)) { >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h >> index cd132c2..3f89607 100644 >> --- a/drivers/gpu/drm/i915/intel_drv.h >> +++ b/drivers/gpu/drm/i915/intel_drv.h >> @@ -430,8 +430,8 @@ struct intel_scaler { >> }; >> >> struct intel_crtc_scaler_state { >> -#define SKL_NUM_SCALERS 2 >> - struct intel_scaler scalers[SKL_NUM_SCALERS]; >> +#define MAX_NUM_SCALERS 2 >> + struct intel_scaler scalers[MAX_NUM_SCALERS]; > > Replaced the gen marker rather than the abreviation. So now we have > what reads as NUM_NUM_SCALERS. > Reverting to original SKL_NUM_SCALERS, Will give more generic name later. -Chris >
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index b7f42c4..8349abe 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3080,7 +3080,7 @@ static void intel_scaler_info(struct seq_file *m, struct intel_crtc *intel_crtc) pipe_config->scaler_state.scaler_users, pipe_config->scaler_state.scaler_id); - for (i = 0; i < SKL_NUM_SCALERS; i++) { + for (i = 0; i < num_scalers; i++) { struct intel_scaler *sc = &pipe_config->scaler_state.scalers[i]; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index be67aee..6eed9c2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -731,6 +731,7 @@ struct intel_device_info { u16 device_id; u8 num_pipes; u8 num_sprites[I915_MAX_PIPES]; + u8 num_scalers[I915_MAX_PIPES]; u8 gen; u16 gen_mask; u8 ring_mask; /* Rings supported by the HW */ diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 185e3bb..ef26fa8 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -282,6 +282,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv) info->num_sprites[PIPE_A] = 2; info->num_sprites[PIPE_B] = 2; info->num_sprites[PIPE_C] = 1; + info->num_scalers[PIPE_A] = 2; + info->num_scalers[PIPE_B] = 2; + info->num_scalers[PIPE_C] = 1; } else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) { for_each_pipe(dev_priv, pipe) info->num_sprites[pipe] = 2; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index bd2c99e..d2023c4 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -15269,14 +15269,8 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe) crtc_state->base.crtc = &intel_crtc->base; /* initialize shared scalers */ - if (INTEL_GEN(dev_priv) >= 9) { - if (pipe == PIPE_C) - intel_crtc->num_scalers = 1; - else - intel_crtc->num_scalers = SKL_NUM_SCALERS; - - skl_init_scalers(dev_priv, intel_crtc, crtc_state); - } + intel_crtc->num_scalers = dev_priv->info.num_scalers[pipe]; + skl_init_scalers(dev_priv, intel_crtc, crtc_state); primary = intel_primary_plane_create(dev_priv, pipe); if (IS_ERR(primary)) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index cd132c2..3f89607 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -430,8 +430,8 @@ struct intel_scaler { }; struct intel_crtc_scaler_state { -#define SKL_NUM_SCALERS 2 - struct intel_scaler scalers[SKL_NUM_SCALERS]; +#define MAX_NUM_SCALERS 2 + struct intel_scaler scalers[MAX_NUM_SCALERS]; /* * scaler_users: keeps track of users requesting scalers on this crtc.
Number of scalers initialization is moved to runtime init from intel_crtc_init for platform specific initialization. Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_device_info.c | 3 +++ drivers/gpu/drm/i915/intel_display.c | 10 ++-------- drivers/gpu/drm/i915/intel_drv.h | 4 ++-- 5 files changed, 9 insertions(+), 11 deletions(-)