On Mon, 2024-04-15 at 05:05 +0000, Shankar, Uma wrote: > > > -----Original Message----- > > From: Luca Coelho <luca@xxxxxxxxx> > > Sent: Friday, April 12, 2024 5:57 PM > > To: Shankar, Uma <uma.shankar@xxxxxxxxx>; Coelho, Luciano > > <luciano.coelho@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; ville.syrjala@xxxxxxxxxxxxxxx; Nikula, Jani > > <jani.nikula@xxxxxxxxx> > > Subject: Re: [PATCH v5 1/4] drm/i915/display: add support for DMC wakelocks > > > > On Fri, 2024-04-12 at 10:30 +0000, Shankar, Uma wrote: > > > > > > > -----Original Message----- > > > > From: Coelho, Luciano <luciano.coelho@xxxxxxxxx> > > > > Sent: Friday, April 12, 2024 3:12 PM > > > > To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > Cc: intel-xe@xxxxxxxxxxxxxxxxxxxxx; Shankar, Uma > > > > <uma.shankar@xxxxxxxxx>; ville.syrjala@xxxxxxxxxxxxxxx; Nikula, Jani > > > > <jani.nikula@xxxxxxxxx> > > > > Subject: [PATCH v5 1/4] drm/i915/display: add support for DMC > > > > wakelocks > > > > > > > > In order to reduce the DC5->DC2 restore time, wakelocks have been > > > > introduced in DMC so the driver can tell it when registers and other > > > > memory areas are going to be accessed and keep their respective blocks > > awake. > > > > > > > > Implement this in the driver by adding the concept of DMC wakelocks. > > > > When the driver needs to access memory which lies inside pre-defined > > > > ranges, it will tell DMC to set the wakelock, access the memory, > > > > then wait for a while and clear the wakelock. > > > > > > > > The wakelock state is protected in the driver with spinlocks to > > > > prevent concurrency issues. > > > > > > Hi Luca, > > > Seems you missed to add the version history. > > > > I've been sending the version history in the cover letter, because I don't think it > > adds any information after it gets to the mainline kernel. The history is lost > > anyway, so the mailing list is a better place to store it (it's unique and meaningful > > there). > > Its matter of preference, but being part of the patch's commit message it stays with it > and can be checked with a git show. Cover letter details gets lost though as it doesn't > end up in the tree. Yes, but the change history in the commit message doesn't tell the full story. If I write, for instance, "In v2: refactor intel_foo_bar()", there's no way to know what it looked like before. That's why I think that, if we want to keep the version history accessible from the git tree, we should have a link to the mailing list discussions, as apparently we do in most cases anyway. > > Bur as I said to someone else before, I can add it to the commit message if you > > think that it's needed. > > Not needed Luca, it was a simple nitpick 😊. You can skip that. Thanks for pointing out, anyway! 😉 I don't want to keep flaming about this, so I'll just try to remember to add it next time, so it doesn't come up again. > > > > > > Anyways, changes look good to me. > > > Reviewed-by: Uma Shankar <uma.shankar@xxxxxxxxx> > > > > Thanks a lot! > > > > Though you didn't review patch 3/4, the one about the module parameter. > > Was that intentional or did you just miss it? > > I think I have reviewed and RB'ed it. The entire series is RB'ed now. Oh, right. "Someone" was not paying attention. I even already had the r-b in the commit message itself. Silly me. Thanks a lot for your reviews! Now I just need to get someone to merge this series, since I don't have commit rights to the repo yet. -- Cheers, Luca.
