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

[Ville Syrjälä]

On Tue, Feb 07, 2017 at 06:51:25PM +0100, Maarten Lankhorst wrote:
> 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?

Hmm. If it doesn't fix anything, then why does it exist?

-- 
Ville Syrjälä
Intel OTC