Re: 830GM still woes

[Thomas Richter <thor@math.tu-berlin.de>]

Am 16.05.2014 18:50, schrieb Daniel Vetter:
> On Fri, May 16, 2014 at 07:04:54PM +0300, Ville Syrjälä wrote:
>> On Fri, May 16, 2014 at 05:09:53PM +0200, Daniel Vetter wrote:
>>> On Fri, May 16, 2014 at 03:41:05PM +0100, Chris Wilson wrote:
>>>> On Fri, May 16, 2014 at 04:02:48PM +0200, Thomas Richter wrote:
>>>>> It's not that I haven't had a patch for it. Really trivial. I wonder
>>>>> what keeps you from adding this to the kernel and just make things
>>>>> working?
>>>>
>>>> You mean this patch?
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>>> index f671aca..3981898 100644
>>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>>> @@ -944,7 +944,7 @@ static const struct intel_watermark_params i915_wm_info = {
>>>>   static const struct intel_watermark_params i830_wm_info = {
>>>>          I855GM_FIFO_SIZE,
>>>>          I915_MAX_WM,
>>>> -       1,
>>>> +       8,
>>>>          2,
>>>>          I830_FIFO_LINE_SIZE
>>>>   };
>>>> @@ -1001,7 +1001,7 @@ static unsigned long intel_calculate_wm(unsigned long clock_in_khz,
>>>>          /* Don't promote wm_size to unsigned... */
>>>>          if (wm_size > (long)wm->max_wm)
>>>>                  wm_size = wm->max_wm;
>>>> -       if (wm_size <= 0)
>>>> +       if (wm_size < (long)wm->default_wm)
>>>>                  wm_size = wm->default_wm;
>>>>          return wm_size;
>>>>   }

For example. There are a couple of possibilities to add this. If this is 
too invasive, it is not too hard to add a similar check to 
i9xx_update_wm for generation 2 only, and enforce the same type of check 
to i830_update_wm

>> On Gen3 the units change to 64B but it still has the same note with
>> the +32B, so I'm not sure what should be done there. I guess it's
>> just a copy paste fumble and maybe the same minimum value should
>> still apply.
>
> Yeah the burst size stuff - afaiu we should select the biggest one
> possible and if that's not working out round the watermark up to match the
> burst size. I didn't spot the +32/-32bytes anywhere though ... I guess
> going with burst_size + 1 should be safest, especially if we make the code
> more flexible to also allow a burst size of 4 for the really high-res
> stuff.

This would be highly appreciated, yes. I currently constantly patch my 
kernel to keep the graphics working, but that's please not a permanent 
solution.

Thank you, and have a nice weekend.

Greetings,
	Thomas