Re: [PATCH 1/4] drm/i915: fix guest virtual PCH detection on non-PCH systems

[Lucas De Marchi <lucas.de.marchi@gmail.com>]

On Wed, Jun 13, 2018 at 11:57 PM Arkadiusz Hiler
<arkadiusz.hiler@intel.com> wrote:
>
> On Wed, Jun 13, 2018 at 10:16:07AM -0700, Lucas De Marchi wrote:
> > On Wed, Jun 13, 2018 at 10:09 AM Lucas De Marchi
> > <lucas.de.marchi@gmail.com> wrote:
> > >
> > > On Wed, Jun 13, 2018 at 1:11 AM Arkadiusz Hiler
> > > <arkadiusz.hiler@intel.com> wrote:
> > > >
> > > > On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> > > > > On Tue, 12 Jun 2018, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote:
> > > > > > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula <jani.nikula@intel.com> wrote:
> > > > > >>
> > > > > >> On Thu, 31 May 2018, Lucas De Marchi <lucas.de.marchi@gmail.com> wrote:
> > > > > >> > On Thu, May 31, 2018 at 02:56:21PM +0300, Jani Nikula wrote:
> > > > > >> >> Virtualized non-PCH systems such as Broxton or Geminilake should use
> > > > > >> >> PCH_NONE to indicate no PCH rather than PCH_NOP. The latter is a
> > > > > >> >> specific case to indicate a PCH system without south display.
> > > > > >> >
> > > > > >> > Then let's go ahead and document it?
> > > > > >>
> > > > > >> Please avoid sending suggestion patches in-reply-to existing
> > > > > >> series. This confused patchwork and screwed up CI for the series, which
> > > > > >> was already a resend just to get CI. :(
> > > > > >
> > > > > > ugh, sorry.  Sometimes just adding a oneline diff is much better than
> > > > > > a hundred words explaining :( ...
> > > > >
> > > > > I feel you, a patch is more precise.
> > > > >
> > > > > > IMO pw is trying to be smarter than it should here or not being smart
> > > > > > enough. Nonetheless I won't do that anymore.
> > > > >
> > > > > I think there were earlier complaints about what it did recognize and
> > > > > what it didn't. I'd be open to only accepting new versions of patches
> > > > > from whoever sent the original patch. Or requiring patch subjects don't
> > > > > start with "Re:". Or both.
> > > >
> > > > No matter what you do here it will misbehave in some scenarios and
> > > > break someone's workflow :<
> > > >
> > > > Originally we required the patches to have X-Mailer set to
> > > > git-send-email, which seems reasonable, but that annoyed people because
> > > > their servers were stripping out those headers.
> > > >
> > > > Other people send out the patches by feeding them to the drafts folder
> > > > through IMAP and then sending them out. This, depending on the
> > > > provider's configuration, also gobbles up a thing or two.
> > > >
> > > > Because of the above I am not sure about trusting "Re:" and matching
> > > > "From:" headers as good enough indicators either.
> > > >
> > > > It just adds more opaque "smartness". I already can foresee questions
> > > > asking "why my v2 was not picked up?" and someone would have to debug it
> > > > down the line.
> > > >
> > > > Was the address different (+XYZ before @)? Has that someone used
> > > > --subject-prefix=re:? Is it an actual bug? Etc.
> > > >
> > > >
> > > > > I was annoyed, but I'm perhaps more annoyed that you can't do this
> > > > > without confusing patchwork. In the end, I wouldn't want to hamper
> > > > > review by blocking something that comes naturally to people.
> > > > >
> > > > > Arek?
> > > >
> > > > Just add the following header:
> > > > "X-Patchwork-Hint: comment"
> > > > to emails that you type out manually.
> > > >
> > > > For mutt it's as easy as adding:
> > > > "my_hdr X-Patchwork-Hint: comment"
> > > > to your .muttrc
> > >
> > > This may not work for the same reasons you pointed out that wouldn't
> > > work if people are sending patches.  Is there a format I can use that
> > > will not trigger patchwork from parsing a _reply_? Does removing the
> > > "--------" help? Under the hood does it use git am --scissors or
> > > similar?
>
> Yeah, it's far for perfection and needs additional effort to set the
> header up. For me it works, but I always send patches via git-send-email
> and always send replies via mutt - I am the simple case.
>
> > Humn... it has its own parser. So if I read
> > https://github.com/dlespiau/patchwork/blob/master/patchwork/parser.py#L36
> > correctly, it should be just a matter of omitting the "diff | ---"
> > lines (or prepending with a "#").
> >
> > It does make things more difficult if the other person would use "git
> > am --scissors" though.
> >
> > Lucas De Marchi
>
> Yes, that is my understanding as well - if you would ommit the "diff
> header" it should not recognize your mail as a patch. But that's yet
> another behavior you have to know upfront.
>
> It's really hard to strike a balance here.
>
> One idea is to optimize for the default case (i.e. behavior of
> git-send-email), sans the known quirks we have seen so far,
> and write this down.
>
> Then, if some patches are getting ignored, this would make a handy
> checklist that can be use for troubleshooting and we can also link to
> it, kindly asking to adhere to a more standard way of sending patches.

Agreed.

Lucas De Marchi

>
> --
> Cheers,
> Arek



-- 
Lucas De Marchi
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx