On Fri, 25 Sep 2020 at 12:38, Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > 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? No. I got another interrupt before I'd refreshed my memory over what the second patch was doing and responding to it. I'll do that now (I don't think it changed much from v1) Dave. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
