<div dir="ltr">On 29 September 2016 at 09:59, Daniel Vetter <<a>daniel@ffwll.ch</a>> wrote:<br>
> On Thu, Sep 29, 2016 at 10:25:09AM +0200, Tomeu Vizoso wrote:<br>
>> On 29 September 2016 at 05:42, Dhinakaran Pandiyan<br>
>> <<a>dhinakaran.pandiyan@intel.com</a><wbr>> wrote:<br>
>> > vgem does not do modeset, looping through non-existent CRTC's while<br>
>> > registering drm_minor in<br>
>> ><br>
>> >         'commit 48c787899882 ("drm: Add API for capturing frame CRCs")'<br>
>> ><br>
>> > caused kernel oops. So, let's add CRC debugfs files<br>
>> > only for those drivers that do modeset.<br>
>> ><br>
>> > Signed-off-by: Dhinakaran Pandiyan <<a>dhinakaran.pandiyan@intel.com</a><wbr>><br>
>> > Cc: Tomeu Vizoso <<a>tomeu.vizoso@collabora.com</a>><br>
>> > Cc: Daniel Vetter <<a>daniel.vetter@ffwll.ch</a>><br>
>> > Cc: Emil Velikov <<a>emil.velikov@collabora.com</a>><br>
>><br>
>> Reviewed-by: Tomeu Vizoso <<a>tomeu.vizoso@collabora.com</a>><br>
><br>
> Applied to drm-misc, thanks.<br>
><br>
>> But I would prefer if drm_for_each_crtc was safe to call in any device<br>
>> regardless of the features that its driver supports.<br>
><br>
> Imo explicit checks are much better, especially in userspace-facing code<br>
> like this (well it's debugfs and not an ioctl, but still). Unrelated rant:<br>
> I think we should throw away the concept of having per-minor debugfs (with<br>
> a symlink for backwards compat). Then we could have put the crc code into<br>
> drm_modeset_register_all, and there would not have been any bug.<br>Fwiw I agree that explicit checks are better.<br>
<br>I keep forgetting that we create a card# node even if the device does not feature DRIVER_MODESET. IMHO if we drop that assumption, we can simplify core DRM and/or some drivers. If userspace is not new enough to know about render nodes it's simply not capable of using new drivers/devices properly and there will be no regressions afaict. Not to mention it the dubious auth requirement even if the device is render only. Alternatively if we can have add a comment why the card# is required that'll be greatly appreciated.<div><br></div><div><div><div><div><div>That said, the patch is a good fix and is</div><div>Reviewed-by: Emil Velikov <<a href="mailto:emil.velikov@collabora.com">emil.velikov@collabora.com</a>><br>
<br>Regards,</div><div>Emil</div><div><br></div><div>P.S. Some of the links on your blog have gone crazy [2]</div><div> <br>
[1] <a href="https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#render-nodes">https://dri.freedesktop.org/docs/drm/gpu/drm-uapi.html#render-nodes</a><br>
[2] <a href="http://blog.ffwll.ch/2016/01/better-markup-for-kernel-gpu-docbook.html">http://blog.ffwll.ch/2016/01/better-markup-for-kernel-gpu-docbook.html</a></div><div><br>
</div></div>
</div></div></div></div>