<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body text="#000000" bgcolor="#FFFFFF"> Hi, <div class="moz-cite-prefix">On Friday 21 July 2017 06:56 PM, Imre Deak wrote: </div> <blockquote type="cite" cite="mid:20170721132653.ws2kc34so4aodaz4@ideak-desk.fi.intel.com"> <pre wrap="">On Fri, Jul 21, 2017 at 06:44:24PM +0530, Mahesh Kumar wrote: </pre> <blockquote type="cite"> <pre wrap="">Hi, On Thursday 20 July 2017 04:20 AM, Imre Deak wrote: </pre> <blockquote type="cite"> <pre wrap="">The crtc state starts out being bzero'd, so no need to clear scaler_users. Also intel_crtc_init_scalers() knows already which platforms have scalers, so no need for the platform check here. Similarly intel_crtc_init_scalers() will init scaler_id as required, so no need to do it here separately. Cc: Chandra Konduru <a class="moz-txt-link-rfc2396E" href="mailto:chandra.konduru@intel.com"><chandra.konduru@intel.com></a> Cc: Matt Roper <a class="moz-txt-link-rfc2396E" href="mailto:matthew.d.roper@intel.com"><matthew.d.roper@intel.com></a> Signed-off-by: Imre Deak <a class="moz-txt-link-rfc2396E" href="mailto:imre.deak@intel.com"><imre.deak@intel.com></a> --- drivers/gpu/drm/i915/intel_display.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 8a38e64b1931..7210f418e9c0 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9132,12 +9132,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc, u64 power_domain_mask; bool active; - if (INTEL_GEN(dev_priv) >= 9) { - intel_crtc_init_scalers(crtc, pipe_config); - - pipe_config->scaler_state.scaler_id = -1; - pipe_config->scaler_state.scaler_users &= ~(1 << SKL_CRTC_INDEX); - } + intel_crtc_init_scalers(crtc, pipe_config); </pre> </blockquote> <pre wrap="">If we are removing gen check, then IMHO we should initialize "scaler_id = -1" even before !crtc->num_scalers check in intel_crtc_init_scalers function, just to make sure scaler state doesn't have wrong value in platforms where we don't have any scalers. </pre> </blockquote> <pre wrap=""> I agree that setting scaler_id to -1 on all platforms is a good idea, but it's unrelated to removing here the GEN check. intel_crtc_init_scalers() is already called for all platforms in intel_crtc_init(). So this change won't make a difference for platforms without scalers: we leave scaler_id zeroed on those already now (they have num_scalers set to 0). This isn't either a problem since we allocate scalers only on GEN9+.</pre> </blockquote> <blockquote type="cite" cite="mid:20170721132653.ws2kc34so4aodaz4@ideak-desk.fi.intel.com"> <pre wrap=""> So we could do what you suggest but imo as a follow-up.</pre> </blockquote> sounds good, But if you are going to write a cleanup patch, will that be possible for you to do following change also. 0 indicates no scaler is used & non-zero +ve value indicates used scaler id. in any case Reviewed-by: Mahesh Kumar <a class="moz-txt-link-rfc2396E" href="mailto:mahesh1.kumar@intel.com"><mahesh1.kumar@intel.com></a> -Mahesh <blockquote type="cite" cite="mid:20170721132653.ws2kc34so4aodaz4@ideak-desk.fi.intel.com"> <pre wrap=""> --Imre </pre> </blockquote> </body> </html>