Hi Dave, On Wed, Sep 23, 2020 at 03:59:04PM +0100, Dave Stevenson wrote: > Hi Maxime > > On Wed, 23 Sep 2020 at 09:40, Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > > > The current CRTC state reset hook in vc4 allocates a vc4_crtc_state > > structure as a drm_crtc_state, and relies on the fact that vc4_crtc_state > > embeds drm_crtc_state as its first member, and therefore can be safely > > casted. > > s/casted/cast > > > However, this is pretty fragile especially since there's no check for this > > in place, and we're going to need to access vc4_crtc_state member at reset > > so this looks like a good occasion to make it more robust. > > > > Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx> > > Tested-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> > > Based on the issue I perceived with the previous patch, I'm happy. I > haven't thought about how the framework handles losing the state, but > that's not the driver's problem. > > There is still an implicit assumption that drm_crtc_state is the first > member from the implementation of to_vc4_crtc_state in vc4_drv.h. To > make it even more robust that could be a container_of instead. I > haven't checked for any other places that make the assumption though. Good catch, I'll send another patch to fix it > Reviewed-by: Dave Stevenson <dave.stevenson@xxxxxxxxxxxxxxx> Does it apply to the second patch as well? Thanks! Maxime _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
