[Intel-gfx] [PATCH RFC] drm/i915: reduce cursor size for GEN5 hardware

[Maarten Lankhorst]

Op 07-02-17 om 16:35 schreef Ville Syrjälä:
> On Tue, Feb 07, 2017 at 05:03:09PM +0200, Martin Peres wrote:
>> On 07/02/17 15:22, Uwe Kleine-König wrote:
>>> Hello,
>>>
>>> On 02/01/2017 03:37 PM, Ville Syrjälä wrote:
>>>> On Wed, Feb 01, 2017 at 01:41:08PM +0100, Maarten Lankhorst wrote:
>>>>> Op 31-01-17 om 20:13 schreef Uwe Kleine-König:
>>>>>> On Tue, Jan 31, 2017 at 10:03:26AM +0100, Maarten Lankhorst wrote:
>>>>>>> Op 31-01-17 om 09:09 schreef Uwe Kleine-König:
>>>>>>> Just curious, does this help?
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>>>>> index ae2c0bb4b2e8..13de4c526ca6 100644
>>>>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>>>>> @@ -1838,7 +1838,7 @@ static uint32_t ilk_compute_cur_wm(const struct intel_crtc_state *cstate,
>>>>>>>  	 * this is necessary to avoid flickering.
>>>>>>>  	 */
>>>>>>>  	int cpp = 4;
>>>>>>> -	int width = pstate->base.visible ? pstate->base.crtc_w : 64;
>>>>>>> +	int width = 256;
>>>>>>>
>>>>>>>  	if (!cstate->base.active)
>>>>>>>  		return 0;
>>>>>>>
>>>>>> On a kernel with this patch applied I cannot reproduce the flickering. I
>>>>>> keep that kernel running but expect that it also fixes the crash.
>>>>> Ok that's good news.
>>>>>
>>>>> Maybe ville or matt can comment whether this patch is the right fix?
>>>> Well, it's just extending the hack even further. The right fix would be
>>>> to fix the wm programming sequence to respect the frame boundaries
>>>> correctly (ie. my old vblank based wm stuff).
>>> so I wonder how this goes forward. The situation seems to be well
>>> understood, but other than testing patches I don't know what to do (and
>>> there is currently no patch to test).
>>>
>>> Best regards
>>> Uwe
>>>
>> The way I understand this is that no-one wants to restrict the 
>> capabilities exposed by the kernel and would like a proper fix for this. 
>> However, I agree with Uwe, given the low priority status of Gen5 (people 
>> would rather work on hw that is used by a lot of people), we should 
>> probably accept the patch proposed by Maarten as it fixes someone's 
>> workflow and does not regress anything meaningful.
> The same code is used for ILK-BDW, so it's not just ILK. And other other
> platform suffers from the same problem of cursor vs. watermarks. It just
> seems that most people are lucky enough to not be seriously affected by
> this problem.
>
> Also it can regress some things, at least theoretically. Power consumption
> with < 256x256 for one, and potentially it could also end up rejecting
> some display modes that previously used to work with smaller cursor
> sizes (or no cursors). That last part may not be 100% true, but I was
> too lazy to go through the math to see if the cursor FIFO could end up
> being the limiting factor in some cases.
>
> I was thinking Maarten's intel_legacy_cursor_update() hack should have
> "fixed" this, but now I'm not sure since it still sets the
> legacy_cursor_update flag in the slow path, and the commit message
> didn't quite manage to tell me what the purpose of this function 
> was supposed to be. The logic for picking the slow path also seems a
> little wonky to me (assuming I deduced the purpose of the function
> correctly).
Hey,

This patch shouldn't fix anything, maybe wait for a vblank on changing to a smaller stride?