[PATCH] drm/mgag200: Increase bandwidth for G200se A rev1

[Jocelyn Falempe]

On 28/07/2023 14:12, Roger Sewell wrote:
> 
> Thomas, Jocelyn,
> 
> JF> I think the culprit is probably this patch:
> JF> https://patchwork.freedesktop.org/patch/486242/
> JF>
> JF> before this patch,
> JF> mgag200_simple_display_pipe_mode_valid() always return MODE_OK
> JF>
> JF> after this patch, it checks the bandwidth limit too.
> 
> It turns out to be more complicated than that - and I still think it is
> something to do with how the two functions
> mgag200_simple_display_pipe_mode_valid and
> mgag200_mode_config_mode_valid are made known to the rest of the drm
> system, i.e. which slots in the various function structures they are put
> in.
> 
> I attach a contiguous excerpt from /var/log/messages, recording what
> happened when I did the following.
> 
> 1. I instrumented the old mgag200 module with printk statements in
>     mgag200_simple_display_pipe_mode_valid and mga_vga_mode_valid and
>     mga_vga_calculate_mode_bandwidth, and also put in a call to the
>     latter in mgag200_simple_display_pipe_mode_valid so that I could see
>     what parameters it had been called with. I then rebooted the system,
>     getting the messages starting at Jul 28 10:42:45 . As you will see,
>     almost every time mgag200_simple_display_pipe_mode_valid is called it
>     is immediately following a return of MODE_OK from mga_vga_mode_valid
>     with the same display parameters - the two exceptions are:
> 
>     a) at line 1156 is when it is called after "fbcon: mgag200drmfb (fb0)
>        is primary device", and
> 
>     b) with the mode actually being set (1440x900) at line 2681 when it
>        of course returns MODE_OK (as that is what it always returns, as
>        you say).
> 
> 2. I then switched to the new mgag200 module similarly instrumented, but
>     with the unique_rev_id increased by 1 to get sufficient bandwidth to
>     make 1440x900 usable. I then rebooted the system, getting the
>     messages starting at Jul 28 11:46:53 . Again, almost every time
>     mgag200_simple_display_pipe_mode_valid is called it is immediately
>     after a return of MODE_OK from mgag200_mode_config_mode_valid, and we
>     still have exception type (a) at line 5672. However, the exception
>     type (b) is no longer present, as at line 6590, on setting the
>     1440x900 mode, there is now a call of mgag200_mode_config_mode_valid
>     preceding the call of mgag200_simple_display_pipe_mode_valid.
> 
> 3. I then modified that mgag200 module by forcing a return of MODE_OK
>     from mgag200_simple_display_pipe_mode_valid and removing the
>     increment to unique_rev_id, so that 1440x900 is no longer "valid"
>     according to the criteria being used. I rebooted, getting the
>     messages starting at Jul 28 12:12:34 . Now at line 11004 we have a
>     failure to set mode immediately followed by a return of MODE_BAD, not
>     from mgag200_simple_display_pipe_mode_valid but from
>     mgag200_mode_config_mode_valid.
> 
> Thus between the old mgag200 module and the new one, there is a change
> in when the mode_config_mode_valid function gets called - there being
> one extra call. So even if one were to revert to
> mgag200_simple_display_pipe_mode_valid just always returning MODE_OK it
> wouldn't fix the problem.
> 
> At present I don't see how the change of behaviour can be anything other
> than to do with the way these function names are passed to the rest of
> the drm system (though of course even if that were reverted, the fact
> that mgag200_simple_display_pipe_mode_valid now tests bandwidth would
> still cause what I want to do to fail).
> 
> Sadly I don't understand how the drm system works, so I'm not sure that
> I can shed any more light - but if there are any more experiments that
> would help, please do let me know.

I think the issue is that in v5.18, the memory check was done on the 
connector mode_valid() callback, and in v6.0, it's done in the 
mode_config mode_valid() callback.

Can you please try the patch attached, I moved the bandwidth check back 
to the connector callback.

Best regards,

-- 

Jocelyn
> 
> Thanks,
> Roger.
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-drm-mgag200-move-the-memory-bandwidth-check-to-the-c.patch
Type: text/x-patch
Size: 4464 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20230801/e0fb658d/attachment.bin>