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

2018-06-19 Thread Lucas De Marchi
On Wed, Jun 13, 2018 at 11:57 PM Arkadiusz Hiler
 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
> >  wrote:
> > >
> > > On Wed, Jun 13, 2018 at 1:11 AM Arkadiusz Hiler
> > >  wrote:
> > > >
> > > > On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> > > > > On Tue, 12 Jun 2018, Lucas De Marchi  
> > > > > wrote:
> > > > > > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  
> > > > > > wrote:
> > > > > >>
> > > > > >> On Thu, 31 May 2018, Lucas De Marchi  
> > > > > >> 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

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

2018-06-14 Thread Arkadiusz Hiler
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
>  wrote:
> >
> > On Wed, Jun 13, 2018 at 1:11 AM Arkadiusz Hiler
> >  wrote:
> > >
> > > On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> > > > On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> > > > > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  
> > > > > wrote:
> > > > >>
> > > > >> On Thu, 31 May 2018, Lucas De Marchi  
> > > > >> 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.

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-06-13 Thread Lucas De Marchi
On Wed, Jun 13, 2018 at 10:09 AM Lucas De Marchi
 wrote:
>
> On Wed, Jun 13, 2018 at 1:11 AM Arkadiusz Hiler
>  wrote:
> >
> > On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> > > On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> > > > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  
> > > > wrote:
> > > >>
> > > >> On Thu, 31 May 2018, Lucas De Marchi  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?

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

>
>
> Lucas De Marchi
>
> >
> > https://github.com/dlespiau/patchwork/commit/148f10115525
> >
> > --
> > Cheers,
> > Arek
>
>
>
> --
> Lucas De Marchi



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


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

2018-06-13 Thread Lucas De Marchi
On Wed, Jun 13, 2018 at 1:11 AM Arkadiusz Hiler
 wrote:
>
> On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> > On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> > > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  wrote:
> > >>
> > >> On Thu, 31 May 2018, Lucas De Marchi  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?


Lucas De Marchi

>
> https://github.com/dlespiau/patchwork/commit/148f10115525
>
> --
> Cheers,
> Arek



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


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

2018-06-13 Thread Arkadiusz Hiler
On Wed, Jun 13, 2018 at 09:49:09AM +0300, Jani Nikula wrote:
> On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> > On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  wrote:
> >>
> >> On Thu, 31 May 2018, Lucas De Marchi  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

https://github.com/dlespiau/patchwork/commit/148f10115525

-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-06-13 Thread Jani Nikula
On Tue, 12 Jun 2018, Lucas De Marchi  wrote:
> On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  wrote:
>>
>> On Thu, 31 May 2018, Lucas De Marchi  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.

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?

BR,
Jani.




>
> Lucas De Marchi
>
>>
>> I'm resending the series, with your documentation patch added, but I'm
>> keeping the extra explanatory text in the last patch. I think it's
>> warranted.
>>
>> BR,
>> Jani.
>>
>>
>> >
>> > -
>> > Subject: [PATCH] drm/i915: document PCH_NOP
>> >
>> > There's a difference between PCH_NONE and PCH_NOP: the former means we
>> > don't have a PCH while in the latter we do, but it doesn't have the
>> > south display.
>> >
>> > Cc: Jani Nikula 
>> > Signed-off-by: Lucas De Marchi 
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index 72150f89f200..aa395a898258 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -631,7 +631,7 @@ enum intel_pch {
>> >   PCH_KBP,/* Kaby Lake PCH */
>> >   PCH_CNP,/* Cannon Lake PCH */
>> >   PCH_ICP,/* Ice Lake PCH */
>> > - PCH_NOP,
>> > + PCH_NOP,/* PCH without south display */
>> >  };
>> >
>> >  enum intel_sbi_destination {
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-06-12 Thread Lucas De Marchi
On Fri, Jun 8, 2018 at 5:34 AM Jani Nikula  wrote:
>
> On Thu, 31 May 2018, Lucas De Marchi  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 :( ... IMO pw is trying to be smarter than
it should here or not being smart enough. Nonetheless I won't do that
anymore.

Lucas De Marchi

>
> I'm resending the series, with your documentation patch added, but I'm
> keeping the extra explanatory text in the last patch. I think it's
> warranted.
>
> BR,
> Jani.
>
>
> >
> > -
> > Subject: [PATCH] drm/i915: document PCH_NOP
> >
> > There's a difference between PCH_NONE and PCH_NOP: the former means we
> > don't have a PCH while in the latter we do, but it doesn't have the
> > south display.
> >
> > Cc: Jani Nikula 
> > Signed-off-by: Lucas De Marchi 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 72150f89f200..aa395a898258 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -631,7 +631,7 @@ enum intel_pch {
> >   PCH_KBP,/* Kaby Lake PCH */
> >   PCH_CNP,/* Cannon Lake PCH */
> >   PCH_ICP,/* Ice Lake PCH */
> > - PCH_NOP,
> > + PCH_NOP,/* PCH without south display */
> >  };
> >
> >  enum intel_sbi_destination {
>
> --
> Jani Nikula, Intel Open Source Graphics Center



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


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

2018-06-08 Thread Jani Nikula
On Thu, 31 May 2018, Lucas De Marchi  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. :(

I'm resending the series, with your documentation patch added, but I'm
keeping the extra explanatory text in the last patch. I think it's
warranted.

BR,
Jani.


>
> -
> Subject: [PATCH] drm/i915: document PCH_NOP
>
> There's a difference between PCH_NONE and PCH_NOP: the former means we
> don't have a PCH while in the latter we do, but it doesn't have the
> south display.
>
> Cc: Jani Nikula 
> Signed-off-by: Lucas De Marchi 
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 72150f89f200..aa395a898258 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -631,7 +631,7 @@ enum intel_pch {
>   PCH_KBP,/* Kaby Lake PCH */
>   PCH_CNP,/* Cannon Lake PCH */
>   PCH_ICP,/* Ice Lake PCH */
> - PCH_NOP,
> + PCH_NOP,/* PCH without south display */
>  };
>  
>  enum intel_sbi_destination {

-- 
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-05-31 Thread Colin Xu

On 05/31/2018 07:56 PM, 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.

Reported-by: Colin Xu 
Cc: Colin Xu 
Reviewed-by: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
  drivers/gpu/drm/i915/i915_drv.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8f002ae22e62..c42e389a27f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -287,7 +287,7 @@ static void intel_detect_pch(struct drm_i915_private 
*dev_priv)
if (WARN_ON(pch_type == PCH_NONE))
pch_type = PCH_NOP;
} else {
-   pch_type = PCH_NOP;
+   pch_type = PCH_NONE;
}
dev_priv->pch_type = pch_type;
dev_priv->pch_id = id;


Tested on BXT gvt-g and got expected behaviour.

Tested-by: Colin Xu 
Reviewed-by: Colin Xu 

--
Best Regards,
Colin Xu

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-05-31 Thread Lucas De Marchi
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?

-
Subject: [PATCH] drm/i915: document PCH_NOP

There's a difference between PCH_NONE and PCH_NOP: the former means we
don't have a PCH while in the latter we do, but it doesn't have the
south display.

Cc: Jani Nikula 
Signed-off-by: Lucas De Marchi 
---
 drivers/gpu/drm/i915/i915_drv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 72150f89f200..aa395a898258 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -631,7 +631,7 @@ enum intel_pch {
PCH_KBP,/* Kaby Lake PCH */
PCH_CNP,/* Cannon Lake PCH */
PCH_ICP,/* Ice Lake PCH */
-   PCH_NOP,
+   PCH_NOP,/* PCH without south display */
 };
 
 enum intel_sbi_destination {
-- 


Feel free to squash something like that if you agree.

Lucas De Marchi



> 
> Reported-by: Colin Xu 
> Cc: Colin Xu 
> Reviewed-by: Ville Syrjälä 
> Signed-off-by: Jani Nikula 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 8f002ae22e62..c42e389a27f3 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -287,7 +287,7 @@ static void intel_detect_pch(struct drm_i915_private 
> *dev_priv)
>   if (WARN_ON(pch_type == PCH_NONE))
>   pch_type = PCH_NOP;
>   } else {
> - pch_type = PCH_NOP;
> + pch_type = PCH_NONE;
>   }
>   dev_priv->pch_type = pch_type;
>   dev_priv->pch_id = id;
> -- 
> 2.11.0
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


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

2018-05-31 Thread Jani Nikula
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.

Reported-by: Colin Xu 
Cc: Colin Xu 
Reviewed-by: Ville Syrjälä 
Signed-off-by: Jani Nikula 
---
 drivers/gpu/drm/i915/i915_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8f002ae22e62..c42e389a27f3 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -287,7 +287,7 @@ static void intel_detect_pch(struct drm_i915_private 
*dev_priv)
if (WARN_ON(pch_type == PCH_NONE))
pch_type = PCH_NOP;
} else {
-   pch_type = PCH_NOP;
+   pch_type = PCH_NONE;
}
dev_priv->pch_type = pch_type;
dev_priv->pch_id = id;
-- 
2.11.0

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx