[Intel-gfx] [PATCH 2/2] drm/i915: Simplify scaler init during CRTC HW readout

[Mahesh Kumar]

Hi,


On Friday 21 July 2017 06:56 PM, Imre Deak wrote:
> On Fri, Jul 21, 2017 at 06:44:24PM +0530, Mahesh Kumar wrote:
>> Hi,
>>
>>
>> On Thursday 20 July 2017 04:20 AM, Imre Deak wrote:
>>> 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 <chandra.konduru at intel.com>
>>> Cc: Matt Roper <matthew.d.roper at intel.com>
>>> Signed-off-by: Imre Deak <imre.deak at intel.com>
>>> ---
>>>    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);
>> 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.
> 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+.
>
> So we could do what you suggest but imo as a follow-up.
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 <mahesh1.kumar at intel.com>

-Mahesh
>
> --Imre

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20170721/1a5437ed/attachment.html>