Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
On Wed, 12 Aug 2015, "Yang, Libin" wrote: > Hi Jani, > >> -Original Message- >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com] >> Sent: Tuesday, August 11, 2015 4:14 PM >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel- >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch >> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in >> modeset >> >> On Tue, 11 Aug 2015, "Yang, Libin" wrote: >> > Hi Jani, >> > >> >> -Original Message- >> >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com] >> >> Sent: Tuesday, August 11, 2015 3:14 PM >> >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel- >> >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch >> >> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in >> >> modeset >> >> >> >> On Tue, 11 Aug 2015, "Yang, Libin" wrote: >> >> > Hi Jani, >> >> > >> >> > Thanks for careful reviewing for all the patches, please >> >> > see my comments. >> >> > >> >> >> -Original Message- >> >> >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com] >> >> >> Sent: Monday, August 10, 2015 8:10 PM >> >> >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel- >> >> >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch >> >> >> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS >> in >> >> >> modeset >> >> >> >> >> >> On Mon, 10 Aug 2015, libin.y...@intel.com wrote: >> >> >> > From: Libin Yang >> >> >> > >> >> >> > When modeset occurs and the TMDS frequency is set to some >> >> >> > speical value, the N/CTS need to be set manually if audio >> >> >> > is playing. >> >> >> >> >> >> When modeset occurs, we disable everything, and then re-enable >> >> >> everything, and notify the audio driver every step of the way. IIUC >> >> >> there should be no audio playing across the modeset, and when >> the >> >> >> modeset is complete and we've set the valid ELD, the audio driver >> >> >> should >> >> >> call the callback from the earlier patches to set the correct audio >> >> >> rate. >> >> >> >> >> >> Why is this patch needed? Please enlighten me. >> >> > >> >> > Please image this scenario: when audio is playing, the gfx driver >> >> > does the modeset. In this situation, after modeset, audio driver >> >> > will do nothing and continue playing. And audio driver will not call >> >> > set_ncts. >> >> >> >> Why does the audio driver not do anything here? Whenever there's >> a >> >> modeset, the graphics driver will disable audio and invalidate the >> ELD, >> >> which should cause an interrupt to notify the audio driver about >> >> this. This is no different from a hot-unplug, in fact I can't see how >> >> the audio driver could even detect whether there's been a hotplug >> or >> >> not >> >> when there's a codec disable/enable. >> > >> > Yes, it will trigger an interrupt to audio driver when unplug >> > and another interrupt for hotplug. But from audio driver, >> > we will not stop the playing and resume the playing based >> > on the hotplug or modeset. The audio is always playing during >> > the hotplug/modeset. >> >> But the whole display, and consequently ELD, might have changed >> during >> the hotplug, why do you assume you can just keep playing?! The >> display >> might even have changed from HDMI to DP or vice versa. > > The eld info changing will not impact the audio playing. > Actually, you can image the monitor as a digital speaker, just > like the headphone to the analog audio. Unplug the speaker > will not impact on the audio playing. Of course , there is a > little difference, when monitor hotplug, in the interrupt, > we may change the codec configuration dynamically. > > And audio driver supply the mechanism to stop the > audio playing when hotplug if the HW requires. > >> >> We've also been putting a lot of effort into not depending on previous >> state when enabling the outputs, for more reliability. We generally >> don't do partial changes, we disable everything and then enable >> again. And now you're suggesting we look at some register which for all >> we know may have been valid for some other display? > > In the patch, it will not depend on previous state, I think. We > read the register value which is based on the state after modeset. Okay, let's have the patches cleaned up and see. It'll be easier and quicker to solicit more review on that than this expanding thread. Please find one more code comment below. > >> >> Timeout. >> >> >> BR, >> Jani. >> >> >> > >> >> >> >> BR, >> >> Jani. >> >> >> >> >> >> > >> >> >> >> >> >> Also some comments in-line, provided you've convinced me first. ;) >> >> >> >> >> >> > Signed-off-by: Libin Yang >> >> >> > --- >> >> >> > drivers/gpu/drm/i915/i915_reg.h| 6 ++ >> >> >> > drivers/gpu/drm/i915/intel_audio.c | 42 >> >> >> ++ >> >> >> > 2 files changed, 48 insertions(+) >> >> >> > >> >> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h >> >> >>
Re: [Intel-gfx] [PATCH 1/2] Revert "drm/i915: Add eDP intermediate frequencies for CHV"
hi Ville,
can you review these patches ?
regards,
Sivakumar
On Friday 31 July 2015 11:32 AM, Sivakumar Thulasimani wrote:
From: "Thulasimani,Sivakumar"
This reverts
commit fe51bfb95c996733150c44d21e1c9f4b6322a326.
Author: Ville Syrjälä
Date: Thu Mar 12 17:10:38 2015 +0200
CHV does not support intermediate frequencies so reverting the
patch that added it in the first place
Signed-off-by: Sivakumar Thulasimani
---
drivers/gpu/drm/i915/intel_dp.c |6 --
1 file changed, 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 44f8a32..d9fb7a8 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -95,9 +95,6 @@ static const int bxt_rates[] = { 162000, 216000, 243000,
27,
324000, 432000, 54 };
static const int skl_rates[] = { 162000, 216000, 27,
324000, 432000, 54 };
-static const int chv_rates[] = { 162000, 202500, 21, 216000,
-243000, 27, 324000, 405000,
-42, 432000, 54 };
static const int default_rates[] = { 162000, 27, 54 };
/**
@@ -1185,9 +1182,6 @@ intel_dp_source_rates(struct drm_device *dev, const int
**source_rates)
} else if (IS_SKYLAKE(dev)) {
*source_rates = skl_rates;
return ARRAY_SIZE(skl_rates);
- } else if (IS_CHERRYVIEW(dev)) {
- *source_rates = chv_rates;
- return ARRAY_SIZE(chv_rates);
}
*source_rates = default_rates;
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts callback
On Wed, 12 Aug 2015, "Yang, Libin" wrote:
> Hi,
>
>> -Original Message-
>> From: Yang, Libin
>> Sent: Tuesday, August 11, 2015 10:41 AM
>> To: Jani Nikula; alsa-de...@alsa-project.org; ti...@suse.de; intel-
>> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
>> Subject: RE: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
>> callback
>>
>> Hi Jani,
>>
>> Thanks for reviewing, please see my comments
>>
>> > -Original Message-
>> > From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>> > Sent: Monday, August 10, 2015 7:46 PM
>> > To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
>> > g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
>> > Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
>> > callback
>> >
>> > On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
>> > > From: Libin Yang
>> > >
>> > > Add the set_ncts callback.
>> > >
>> > > With the callback, audio driver can trigger
>> > > i915 driver to set the proper N/CTS
>> > > based on different sample rates.
>> > >
>> > > Signed-off-by: Libin Yang
>> > > ---
>> > > include/drm/i915_component.h | 2 ++
>> > > 1 file changed, 2 insertions(+)
>> > >
>> > > diff --git a/include/drm/i915_component.h
>> > b/include/drm/i915_component.h
>> > > index c9a8b64..7305881 100644
>> > > --- a/include/drm/i915_component.h
>> > > +++ b/include/drm/i915_component.h
>> > > @@ -33,6 +33,8 @@ struct i915_audio_component {
>> > > void (*put_power)(struct device *);
>> > > void (*codec_wake_override)(struct device *, bool
>> > enable);
>> > > int (*get_cdclk_freq)(struct device *);
>> > > +int (*set_ncts)(struct device *, int port, int
>> > > dev_entry,
>> > > +int rate);
>> >
>> > I'd rather call this set_audio_rate or similar, and dropping the
>> > references to N and CTS. The caller does not need to know.
>>
>> But it seems the set_audio_rate will confuse the user. From the
>> literal meaning, it is setting the rate of audio, such as sample rate,
>> which will make audio driver developers confused.
>> And the function is just setting N/CTS which is defined from
>> HDMI SPEC.
>
> What about the name sync_audio_rate()?
I'm fine with that.
BR,
Jani.
>
>>
>> BTW: your comment reminds me that get_cdclk_freq() seems
>> to need change the name as cdclk is not in the open spec.
>>
>> >
>> > I'm also not fond of adding a dev_entry parameter and leaving it
>> > unused. I do not think we know specifically how we're going to
>> identify
>> > MST sinks, and the interface may need to be changed anyway. Let's
>> > force
>> > an update in the caller side as well when there's actually sensible
>> > support in our side.
>>
>> The device entry is a concept in HDA spec. For driver implementation,
>> I think we can use an int variable or a struct device to represent it.
>> A int variable is an easy way. There is some place in audio driver using
>> int variable to represent the deivce entry. And audio driver will not
>> care how gfx driver supports the DP1.2 MST, I mean audio driver will
>> not know the structures inside gfx driver.
>>
>> I will remove this parameter in this version if you are thinking the
>> MST interface is indeterminate.
>>
>> BTW: do you know when gfx will normally support MST?
>>
>> Best Regards,
>> Libin
>>
>> >
>> > BR,
>> > Jani.
>> >
>> > > } *ops;
>> > > };
>> > >
>> > > --
>> > > 1.9.1
>> > >
>> > > ___
>> > > Intel-gfx mailing list
>> > > Intel-gfx@lists.freedesktop.org
>> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >
>> > --
>> > Jani Nikula, Intel Open Source Technology Center
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix checksum write for automated test reply
Hi Daniel,
any comments for the patch below ?
regards,
Sivakumar
On Friday 07 August 2015 03:14 PM, Sivakumar Thulasimani wrote:
From: "Thulasimani,Sivakumar"
DP spec requires the checksum of the last block read to be written
when replying to TEST_EDID_READ. This patch fixes the current code
to do the same.
v2: removed loop for jumping blocks and performed direct addition
as recommended by Daniel
Signed-off-by: Sivakumar Thulasimani
---
drivers/gpu/drm/i915/intel_dp.c |9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index f1b9f93..fa6e202 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4090,9 +4090,16 @@ static uint8_t intel_dp_autotest_edid(struct intel_dp
*intel_dp)
intel_dp->aux.i2c_defer_count);
intel_dp->compliance_test_data = INTEL_DP_RESOLUTION_FAILSAFE;
} else {
+ struct edid *block = intel_connector->detect_edid;
+
+ /* We have to write the checksum
+* of the last block read
+*/
+ block += intel_connector->detect_edid->extensions;
+
if (!drm_dp_dpcd_write(&intel_dp->aux,
DP_TEST_EDID_CHECKSUM,
- &intel_connector->detect_edid->checksum,
+ &block->checksum,
1))
DRM_DEBUG_KMS("Failed to write EDID checksum\n");
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Bypass debug message if scalers are not requested
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 7070 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -1 302/302 301/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT 283/283 283/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@rcs-wf_vblank-vs-dpms-interruptible PASS(1) DMESG_WARN(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts callback
Hi,
> -Original Message-
> From: Yang, Libin
> Sent: Tuesday, August 11, 2015 10:41 AM
> To: Jani Nikula; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: RE: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
> callback
>
> Hi Jani,
>
> Thanks for reviewing, please see my comments
>
> > -Original Message-
> > From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> > Sent: Monday, August 10, 2015 7:46 PM
> > To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> > g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> > Subject: Re: [Intel-gfx] [PATCH v2 1/4] drm/i915: Add audio set_ncts
> > callback
> >
> > On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> > > From: Libin Yang
> > >
> > > Add the set_ncts callback.
> > >
> > > With the callback, audio driver can trigger
> > > i915 driver to set the proper N/CTS
> > > based on different sample rates.
> > >
> > > Signed-off-by: Libin Yang
> > > ---
> > > include/drm/i915_component.h | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/include/drm/i915_component.h
> > b/include/drm/i915_component.h
> > > index c9a8b64..7305881 100644
> > > --- a/include/drm/i915_component.h
> > > +++ b/include/drm/i915_component.h
> > > @@ -33,6 +33,8 @@ struct i915_audio_component {
> > > void (*put_power)(struct device *);
> > > void (*codec_wake_override)(struct device *, bool
> > enable);
> > > int (*get_cdclk_freq)(struct device *);
> > > + int (*set_ncts)(struct device *, int port, int dev_entry,
> > > + int rate);
> >
> > I'd rather call this set_audio_rate or similar, and dropping the
> > references to N and CTS. The caller does not need to know.
>
> But it seems the set_audio_rate will confuse the user. From the
> literal meaning, it is setting the rate of audio, such as sample rate,
> which will make audio driver developers confused.
> And the function is just setting N/CTS which is defined from
> HDMI SPEC.
What about the name sync_audio_rate()?
>
> BTW: your comment reminds me that get_cdclk_freq() seems
> to need change the name as cdclk is not in the open spec.
>
> >
> > I'm also not fond of adding a dev_entry parameter and leaving it
> > unused. I do not think we know specifically how we're going to
> identify
> > MST sinks, and the interface may need to be changed anyway. Let's
> > force
> > an update in the caller side as well when there's actually sensible
> > support in our side.
>
> The device entry is a concept in HDA spec. For driver implementation,
> I think we can use an int variable or a struct device to represent it.
> A int variable is an easy way. There is some place in audio driver using
> int variable to represent the deivce entry. And audio driver will not
> care how gfx driver supports the DP1.2 MST, I mean audio driver will
> not know the structures inside gfx driver.
>
> I will remove this parameter in this version if you are thinking the
> MST interface is indeterminate.
>
> BTW: do you know when gfx will normally support MST?
>
> Best Regards,
> Libin
>
> >
> > BR,
> > Jani.
> >
> > > } *ops;
> > > };
> > >
> > > --
> > > 1.9.1
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
Hi Jani,
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Tuesday, August 11, 2015 4:14 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
> modeset
>
> On Tue, 11 Aug 2015, "Yang, Libin" wrote:
> > Hi Jani,
> >
> >> -Original Message-
> >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> >> Sent: Tuesday, August 11, 2015 3:14 PM
> >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> >> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
> >> modeset
> >>
> >> On Tue, 11 Aug 2015, "Yang, Libin" wrote:
> >> > Hi Jani,
> >> >
> >> > Thanks for careful reviewing for all the patches, please
> >> > see my comments.
> >> >
> >> >> -Original Message-
> >> >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> >> >> Sent: Monday, August 10, 2015 8:10 PM
> >> >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> >> >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> >> >> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS
> in
> >> >> modeset
> >> >>
> >> >> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> >> >> > From: Libin Yang
> >> >> >
> >> >> > When modeset occurs and the TMDS frequency is set to some
> >> >> > speical value, the N/CTS need to be set manually if audio
> >> >> > is playing.
> >> >>
> >> >> When modeset occurs, we disable everything, and then re-enable
> >> >> everything, and notify the audio driver every step of the way. IIUC
> >> >> there should be no audio playing across the modeset, and when
> the
> >> >> modeset is complete and we've set the valid ELD, the audio driver
> >> >> should
> >> >> call the callback from the earlier patches to set the correct audio
> >> >> rate.
> >> >>
> >> >> Why is this patch needed? Please enlighten me.
> >> >
> >> > Please image this scenario: when audio is playing, the gfx driver
> >> > does the modeset. In this situation, after modeset, audio driver
> >> > will do nothing and continue playing. And audio driver will not call
> >> > set_ncts.
> >>
> >> Why does the audio driver not do anything here? Whenever there's
> a
> >> modeset, the graphics driver will disable audio and invalidate the
> ELD,
> >> which should cause an interrupt to notify the audio driver about
> >> this. This is no different from a hot-unplug, in fact I can't see how
> >> the audio driver could even detect whether there's been a hotplug
> or
> >> not
> >> when there's a codec disable/enable.
> >
> > Yes, it will trigger an interrupt to audio driver when unplug
> > and another interrupt for hotplug. But from audio driver,
> > we will not stop the playing and resume the playing based
> > on the hotplug or modeset. The audio is always playing during
> > the hotplug/modeset.
>
> But the whole display, and consequently ELD, might have changed
> during
> the hotplug, why do you assume you can just keep playing?! The
> display
> might even have changed from HDMI to DP or vice versa.
The eld info changing will not impact the audio playing.
Actually, you can image the monitor as a digital speaker, just
like the headphone to the analog audio. Unplug the speaker
will not impact on the audio playing. Of course , there is a
little difference, when monitor hotplug, in the interrupt,
we may change the codec configuration dynamically.
And audio driver supply the mechanism to stop the
audio playing when hotplug if the HW requires.
>
> We've also been putting a lot of effort into not depending on previous
> state when enabling the outputs, for more reliability. We generally
> don't do partial changes, we disable everything and then enable
> again. And now you're suggesting we look at some register which for all
> we know may have been valid for some other display?
In the patch, it will not depend on previous state, I think. We
read the register value which is based on the state after modeset.
>
> Timeout.
>
>
> BR,
> Jani.
>
>
> >
> >>
> >> BR,
> >> Jani.
> >>
> >>
> >> >
> >> >>
> >> >> Also some comments in-line, provided you've convinced me first. ;)
> >> >>
> >> >> > Signed-off-by: Libin Yang
> >> >> > ---
> >> >> > drivers/gpu/drm/i915/i915_reg.h| 6 ++
> >> >> > drivers/gpu/drm/i915/intel_audio.c | 42
> >> >> ++
> >> >> > 2 files changed, 48 insertions(+)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> >> b/drivers/gpu/drm/i915/i915_reg.h
> >> >> > index da2d128..85f3beb 100644
> >> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> >> > @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
> >> >> >
> _HSW_AUD_DIG_CNVT_2)
> >> >> > #define DIP_PORT_SEL_MASK0x3
> >> >> >
> >> >> > +#define _HSW_AUD_STR
Re: [Intel-gfx] [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.
> On Tue, 2015-08-11 at 07:05 +, Zhang, Xiong Y wrote: > > > -Original Message- > > > From: Vivi, Rodrigo > > > Sent: Saturday, August 8, 2015 8:34 AM > > > To: intel-gfx@lists.freedesktop.org > > > Cc: Vivi, Rodrigo; Zhang, Xiong Y > > > Subject: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes. > > > > > > DDI-A and DDI-E shares the 4 lanes. So when DDI-E is present we need > > > to configure lane count propperly for both. > > > > > > This was based on Sonika's > > > [PATCH] drm/i915/skl: Select DDIA lane capability based upon vbt > > > > > > Credits-to: Sonika Jindal > > > Cc: Xiong Zhang > > > Signed-off-by: Rodrigo Vivi > > > --- > > > drivers/gpu/drm/i915/intel_ddi.c | 12 ++-- > > > drivers/gpu/drm/i915/intel_dp.c | 8 +--- > > > 2 files changed, 15 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > > b/drivers/gpu/drm/i915/intel_ddi.c > > > index 110d546..557cecf 100644 > > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > > @@ -3178,7 +3178,15 @@ void intel_ddi_init(struct drm_device *dev, > > > enum port port) > > > struct intel_digital_port *intel_dig_port; > > > struct intel_encoder *intel_encoder; > > > struct drm_encoder *encoder; > > > - bool init_hdmi, init_dp; > > > + bool init_hdmi, init_dp, ddi_e_present; > > > + > > > + /* > > > + * On SKL we don't have a way to detect DDI-E so we rely > > > on VBT. > > > + */ > > > + ddie_present = IS_SKYLAKE(dev) && > > > + (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp > > > || > > > + dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi > > > || > > > + dev_priv > > > ->vbt.ddi_port_info[PORT_E].supports_hdmi); > > [Zhang, Xiong Y] ddie_present should be ddi_e_present > > > > > > init_hdmi = (dev_priv > > > ->vbt.ddi_port_info[port].supports_dvi || > > >dev_priv > > > ->vbt.ddi_port_info[port].supports_hdmi); > > > @@ -3210,7 +3218,7 @@ void intel_ddi_init(struct drm_device *dev, > > > enum port port) > > > intel_dig_port->port = port; > > > intel_dig_port->saved_port_bits = > > > I915_READ(DDI_BUF_CTL(port)) & > > > (DDI_BUF_PORT_REVERSAL | > > > -DDI_A_4_LANES); > > > +ddi_e_present ? 0 : > > > DDI_A_4_LANES); > > [Zhang, Xiong Y] Sonika's patch will set DDI-A to 4 lanes when DDI-E > > doesn't exist, I think your patch will do nothing. > > Actually DDI_A_4_LANES is being already set unconditionally, so Sonika's > patch has no effect. [Zhang, Xiong Y] No. Sonika's patch set DDI_A_4_LANES under many conditions. + if (IS_SKYLAKE(dev) && port == PORT_A + && !(val & DDI_BUF_CTL_ENABLE) + && !dev_priv->vbt.ddi_e_used) + I915_WRITE(DDI_BUF_CTL(port), val | DDI_A_4_LANES) > > saved_port_bits goes to intel_dp->DP that goes to DDI_BUF_CTL and also it is > used to calculate the number of lanes. > > With this patch we stop setting DDI_A_4_LANES when ddi_e is present so > DDI-A keeps with 2 lanes and let other 2 lanes for DDI-E [Zhang, Xiong Y] Yes, this patch will clear DDI_A_4_LANES when ddi_e is present. But this patch won't set DDI_A_4_LANES under following conditions which is purpose for Sonika patch 1. Bios fail to driver eDP and doesn't enable DDI_A buffer 2. Bios clear DDI_A_4_LANES 3. DDI_E isn't present thanks > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic: Call ww_acquire_done after check phase is complete
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 7067 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -2 302/302 300/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -1 283/283 282/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@flip-vs-rmfb-interruptible PASS(1) DMESG_WARN(1) *ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1) *BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] Intel-kms in Linux-4.2rc causes regression due to dithering always on.
On 08/07/2015 09:14 AM, Daniel Vetter wrote: On Fri, Aug 07, 2015 at 12:45:52AM +0200, Mario Kleiner wrote: On 08/07/2015 12:12 AM, Daniel Vetter wrote: On Thu, Aug 6, 2015 at 11:56 PM, Mario Kleiner wrote: Hi Daniel and all, since Linux 4.2 (tested with rc4), i think this commit d328c9d78d64ca11e744fe227096990430a88477 "drm/i915: Select starting pipe bpp irrespective or the primary plane" causes trouble for me and my users, as tested on Intel HD Ironlake and Ivy Bridge with MiniDP->Singlelink-DVI adapter -> Measurement device. Afaics it causes dithering to always be enabled on a regular 8bpc framebuffer, even when outputting to a 8 bpc DVI-D output, and that dithering causes my display measurement equipment and other special display devices used for neuro-science and medical applications to fail. This equipment requires an identity passthrough of 8 bpc framebuffer pixels to the digital outputs, iow. dithering off. Log output on Linux 4.1 (good): Aug 1 06:39:26 twisty kernel: [ 154.175394] [drm:connected_sink_compute_bpp] [CONNECTOR:35:HDMI-A-1] checking for sink bpp constrains Aug 1 06:39:26 twisty kernel: [ 154.175396] [drm:intel_hdmi_compute_config] picking bpc to 8 for HDMI output Aug 1 06:39:26 twisty kernel: [ 154.175397] [drm:intel_hdmi_compute_config] forcing pipe bpc to 24 for HDMI Aug 1 06:39:26 twisty kernel: [ 154.175400] [drm:ironlake_check_fdi_lanes] checking fdi config on pipe A, lanes 1 Aug 1 06:39:26 twisty kernel: [ 154.175402] [drm:intel_modeset_pipe_config] plane bpp: 24, pipe bpp: 24, dithering: 0 Aug 1 06:39:26 twisty kernel: [ 154.175403] [drm:intel_dump_pipe_config] [CRTC:20][modeset] config for pipe A Aug 1 06:39:26 twisty kernel: [ 154.175404] [drm:intel_dump_pipe_config] cpu_transcoder: A Aug 1 06:39:26 twisty kernel: [ 154.175405] [drm:intel_dump_pipe_config] pipe bpp: 24, dithering: 0 Log output on Linux 4.2-rc4 (bad): Aug 1 06:21:31 twisty kernel: [ 200.924831] [drm:connected_sink_compute_bpp] [CONNECTOR:36:HDMI-A-1] checking for sink bpp constrains Aug 1 06:21:31 twisty kernel: [ 200.924832] [drm:connected_sink_compute_bpp] clamping display bpp (was 36) to default limit of 24 Aug 1 06:21:31 twisty kernel: [ 200.924834] [drm:intel_hdmi_compute_config] picking bpc to 8 for HDMI output Aug 1 06:21:31 twisty kernel: [ 200.924835] [drm:intel_hdmi_compute_config] forcing pipe bpc to 24 for HDMI Aug 1 06:21:31 twisty kernel: [ 200.924838] [drm:ironlake_check_fdi_lanes] checking fdi config on pipe A, lanes 1 Aug 1 06:21:31 twisty kernel: [ 200.924840] [drm:intel_modeset_pipe_config] plane bpp: 36, pipe bpp: 24, dithering: 1 Aug 1 06:21:31 twisty kernel: [ 200.924841] [drm:intel_dump_pipe_config] [CRTC:21][modeset] config 880131a5c800 for pipe A Aug 1 06:21:31 twisty kernel: [ 200.924842] [drm:intel_dump_pipe_config] cpu_transcoder: A Aug 1 06:21:31 twisty kernel: [ 200.924843] [drm:intel_dump_pipe_config] pipe bpp: 24, dithering: 1 Ideas what to do about this? Well I somehow assumed the dither bit would be sane and not wreak havoc with the lower bits when they would fit into the final bpc pipe mode ... Can you confirm with your equipment that we seem to be doing 8bpc->6bpc dithering on the 8bpc sink? It will need a bit of work to find this out when i'm back in the lab. So far i just know something bad is happening to the signal and i assume it's the dithering, because the visual error pattern of messiness looks like that caused by dithering. E.g., on a static framebuffer i see some repeating pattern over the screen, but the pattern changes with every OpenGL bufferswap, even if i swap to the same fb content, as if the swap triggers some change of the spatial dither pattern (assuming PIPECONF_DITHER_TYPE_SP = spatial dithering?) If that's the case we simply limit to only ever dither when the sink is 6bpc, and not in any other case. -Daniel That would be an improvement for my immediate problem if that works. But assuming we have 10 bpc framebuffers at some point, dithering 10 bpc -> 8 bpc would also have some practical use. Probably some dynamic check would be good, a la if there is a mismatch between the max(bpc) over all active planes and the supported depth of the sink then dither? It's not clear to me where the dithering happens on intel hw. I'd expected that with a 24 bpp framebuffer feeding into a 24 bpp pipe, dithering simply wouldn't do anything even if enabled. Yeah my assumption was that if you run the pipe at a given bpc it will just pass through anything that fits and only dither the additional bits. But obviously that's not how the hardware works ... The problem with adaptive schemes is that we have multiple planes nowadays and they might all run at different depths. And dither seems to be happening at the pipe/overall level (at least there's only one bit). Of course this wouldn't be a problem if the thing wouldn't mangle bits which should pass! Anyway if we can confirm this I think dithering for only 6bpc
Re: [Intel-gfx] [PATCH 4/4] drm/i915: Use CPU mapping for userspace dma-buf mmap()
On Tue, Aug 11, 2015 at 05:59:23PM -0300, Tiago Vignatti wrote:
> Userspace is the one in charge of flush CPU by wrapping mmap with
> begin{,end}_cpu_access.
>
> v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix
> return
> before transferring ownership when mmap fails.
> v3: Fix return values.
>
> Signed-off-by: Tiago Vignatti
> ---
> drivers/gpu/drm/i915/i915_gem_dmabuf.c | 18 +-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> index 8447ba4..8b87c86 100644
> --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> @@ -193,7 +193,23 @@ static void i915_gem_dmabuf_kunmap(struct dma_buf
> *dma_buf, unsigned long page_n
>
> static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct
> vm_area_struct *vma)
> {
> - return -EINVAL;
> + struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
> + int ret;
> +
> + if (obj->base.size < vma->vm_end - vma->vm_start)
> + return -EINVAL;
> +
> + if (WARN_ON(!obj->base.filp))
> + return -ENODEV;
That's user triggerable, mmap(dma_buf_export(userptr)), so remove the
WARN.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 13/13] drm/i915: Handle return value in intel_pin_and_fence_fb_obj, v2.
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 7059 -Summary- Platform Delta drm-intel-nightly Series Applied ILK 302/302 302/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -1 283/283 282/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 4/5] tests: Add kms_mmap_write_crc for cache coherency tests
This program can be used to detect when the writes don't land in scanout due
cache incoherency. Although this seems a problem inherently of non-LCC machines
("Atom"), this particular test catches a cache dirt on scanout on LLC machines
as well. It's inspired in Ville's kms_pwrite_crc.c and can be used also to test
the correctness of the driver's begin_cpu_access (TODO end_cpu_access).
To see the need for flush, one has to run this same binary a few times cause
it's not 100% reproducible (in my Core machine it's always possible to catch
the problem by running it at most 5 times).
Signed-off-by: Tiago Vignatti
---
tests/.gitignore | 1 +
tests/Makefile.sources | 1 +
tests/kms_mmap_write_crc.c | 236 +
3 files changed, 238 insertions(+)
create mode 100644 tests/kms_mmap_write_crc.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 5bc4a58..9ba1e48 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -140,6 +140,7 @@ kms_force_connector
kms_frontbuffer_tracking
kms_legacy_colorkey
kms_mmio_vs_cs_flip
+kms_mmap_write_crc
kms_pipe_b_c_ivb
kms_pipe_crc_basic
kms_plane
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 5b2072e..31c5508 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -163,6 +163,7 @@ TESTS_progs = \
kms_3d \
kms_fence_pin_leak \
kms_force_connector \
+ kms_mmap_write_crc \
kms_pwrite_crc \
kms_sink_crc_basic \
prime_udl \
diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
new file mode 100644
index 000..e24d535
--- /dev/null
+++ b/tests/kms_mmap_write_crc.c
@@ -0,0 +1,236 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Tiago Vignatti
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "igt_kms.h"
+#include "intel_chipset.h"
+#include "ioctl_wrappers.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION(
+ "Use the display CRC support to validate mmap write to an already uncached
future scanout buffer.");
+
+typedef struct {
+ int drm_fd;
+ igt_display_t display;
+ struct igt_fb fb[2];
+ igt_output_t *output;
+ igt_plane_t *primary;
+ enum pipe pipe;
+ igt_crc_t ref_crc;
+ igt_pipe_crc_t *pipe_crc;
+ uint32_t devid;
+} data_t;
+
+int dma_buf_fd;
+
+static char *dmabuf_mmap_framebuffer(int drm_fd, struct igt_fb *fb)
+{
+ char *ptr = NULL;
+
+ dma_buf_fd = prime_handle_to_fd(drm_fd, fb->gem_handle);
+ igt_assert(errno == 0);
+
+ ptr = mmap(NULL, fb->size, PROT_READ | PROT_WRITE, MAP_SHARED,
dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+
+ return ptr;
+}
+
+static void test_begin_access(data_t *data)
+{
+ igt_display_t *display = &data->display;
+ igt_output_t *output = data->output;
+ struct igt_fb *fb = &data->fb[1];
+ drmModeModeInfo *mode;
+ cairo_t *cr;
+ char *ptr;
+ uint32_t caching;
+ void *buf;
+ igt_crc_t crc;
+
+ mode = igt_output_get_mode(output);
+
+ /* create a non-white fb where we can write later */
+ igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+ DRM_FORMAT_XRGB, LOCAL_DRM_FORMAT_MOD_NONE, fb);
+
+ ptr = dmabuf_mmap_framebuffer(data->drm_fd, fb);
+
+ cr = igt_get_cairo_ctx(data->drm_fd, fb);
+ igt_paint_test_pattern(cr, fb->width, fb->height);
+ cairo_destroy(cr);
+
+ /* flip to it to make it UC/WC and fully flushed */
+ igt_plane_set_fb(data->primary, fb);
+ igt_display_commit(display);
+
+ /* flip back the original white buffer */
+ igt_plane_set_fb(data->primary, &data->fb[0]);
+ igt_display_commit(display);
+
+ /* make sure cac
[Intel-gfx] [PATCH i-g-t 5/5] tests/kms_mmap_write_crc: Demonstrate the need for end_cpu_access
It requires i915 changes to add end_cpu_access().
Signed-off-by: Tiago Vignatti
---
tests/kms_mmap_write_crc.c | 63 --
1 file changed, 55 insertions(+), 8 deletions(-)
diff --git a/tests/kms_mmap_write_crc.c b/tests/kms_mmap_write_crc.c
index e24d535..59ac9e7 100644
--- a/tests/kms_mmap_write_crc.c
+++ b/tests/kms_mmap_write_crc.c
@@ -67,6 +67,24 @@ static char *dmabuf_mmap_framebuffer(int drm_fd, struct
igt_fb *fb)
return ptr;
}
+static void dmabuf_sync_start(void)
+{
+ struct dma_buf_sync sync_start;
+
+ memset(&sync_start, 0, sizeof(sync_start));
+ sync_start.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
+ do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_start);
+}
+
+static void dmabuf_sync_end(void)
+{
+ struct dma_buf_sync sync_end;
+
+ memset(&sync_end, 0, sizeof(sync_end));
+ sync_end.flags = DMA_BUF_SYNC_END | DMA_BUF_SYNC_RW;
+ do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_end);
+}
+
static void test_begin_access(data_t *data)
{
igt_display_t *display = &data->display;
@@ -103,14 +121,11 @@ static void test_begin_access(data_t *data)
caching = gem_get_caching(data->drm_fd, fb->gem_handle);
igt_assert(caching == I915_CACHING_NONE || caching ==
I915_CACHING_DISPLAY);
- // Uncomment the following for flush and the crc check next passes. It
- // requires the kernel counter-part of it implemented obviously.
- // {
- // struct dma_buf_sync sync_start;
- // memset(&sync_start, 0, sizeof(sync_start));
- // sync_start.flags = DMA_BUF_SYNC_START | DMA_BUF_SYNC_RW;
- // do_ioctl(dma_buf_fd, DMA_BUF_IOCTL_SYNC, &sync_start);
- // }
+ /*
+* firstly demonstrate the need for DMA_BUF_SYNC_START
("begin_cpu_access")
+*/
+
+ dmabuf_sync_start();
/* use dmabuf pointer to make the other fb all white too */
buf = malloc(fb->size);
@@ -126,6 +141,38 @@ static void test_begin_access(data_t *data)
/* check that the crc is as expected, which requires that caches got
flushed */
igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
igt_assert_crc_equal(&crc, &data->ref_crc);
+
+ /*
+* now demonstrates the need for DMA_BUF_SYNC_END ("end_cpu_access")
+*/
+
+ /* start over, writing non-white to the fb again and flip to it to make
it
+* fully flushed */
+ cr = igt_get_cairo_ctx(data->drm_fd, fb);
+ igt_paint_test_pattern(cr, fb->width, fb->height);
+ cairo_destroy(cr);
+
+ igt_plane_set_fb(data->primary, fb);
+ igt_display_commit(display);
+
+ /* sync start, to move to CPU domain */
+ dmabuf_sync_start();
+
+ /* use dmabuf pointer in the same fb to make it all white */
+ buf = malloc(fb->size);
+ igt_assert(buf != NULL);
+ memset(buf, 0xff, fb->size);
+ memcpy(ptr, buf, fb->size);
+ free(buf);
+
+ /* there's an implicit flush in set_fb() as well (to set to the GTT
domain),
+* so if we don't do it and instead write directly into the fb as it is
the
+* scanout, that should demonstrate the need for end_cpu_access */
+ dmabuf_sync_end();
+
+ /* check that the crc is as expected, which requires that caches got
flushed */
+ igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
+ igt_assert_crc_equal(&crc, &data->ref_crc);
}
static bool prepare_crtc(data_t *data)
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v3] mmap on the dma-buf directly
Hi,
The idea is to create a GEM bo in one process and pass the prime handle of the
it to another process, which in turn uses the handle only to map and write.
This could be useful for Chrome OS architecture, where the Web content
("unpriviledged process") maps and CPU-draws a buffer, which was previously
allocated in the GPU process ("priviledged process").
In v2, I've added a patch that Daniel kindly drafted to allow the
unpriviledged process flush through a prime fd. In v3, I've fixed a few
concerns and then added end_cpu_access to i915.
To validate all this I'm using igt, and sending the tests for review now.
Please take a look.
Tiago
Daniel Thompson (1):
drm: prime: Honour O_RDWR during prime-handle-to-fd
Daniel Vetter (1):
dma-buf: Add ioctls to allow userspace to flush
Tiago Vignatti (2):
drm/i915: Implement end_cpu_access
drm/i915: Use CPU mapping for userspace dma-buf mmap()
drivers/dma-buf/dma-buf.c | 47 ++
drivers/gpu/drm/drm_prime.c| 10 +++-
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 28 +++-
include/uapi/drm/drm.h | 1 +
include/uapi/linux/dma-buf.h | 39
5 files changed, 117 insertions(+), 8 deletions(-)
create mode 100644 include/uapi/linux/dma-buf.h
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 3/5] prime_mmap: Add basic tests to write in a bo using CPU
This patch adds test_correct_cpu_write, which maps the texture buffer through a
prime fd and then writes directly to it using the CPU. It stresses the driver
to guarantee cache synchronization among the different domains.
This test also adds test_forked_cpu_write, which creates the GEM bo in one
process and pass the prime handle of the it to another process, which in turn
uses the handle only to map and write. Grossly speaking this test simulates
Chrome OS architecture, where the Web content ("unpriviledged process") maps
and CPU-draws a buffer, which was previously allocated in the GPU process
("priviledged process").
This requires kernel modifications (Daniel Thompson's "drm: prime: Honour
O_RDWR during prime-handle-to-fd").
Signed-off-by: Tiago Vignatti
---
lib/ioctl_wrappers.c | 5 +++-
tests/prime_mmap.c | 65
2 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 53bd635..941fa66 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1125,6 +1125,9 @@ void gem_require_ring(int fd, int ring_id)
/* prime */
+#ifndef DRM_RDWR
+#define DRM_RDWR O_RDWR
+#endif
/**
* prime_handle_to_fd:
* @fd: open i915 drm file descriptor
@@ -1142,7 +1145,7 @@ int prime_handle_to_fd(int fd, uint32_t handle)
memset(&args, 0, sizeof(args));
args.handle = handle;
- args.flags = DRM_CLOEXEC;
+ args.flags = DRM_CLOEXEC | DRM_RDWR;
args.fd = -1;
do_ioctl(fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
index dc59e8f..ad91371 100644
--- a/tests/prime_mmap.c
+++ b/tests/prime_mmap.c
@@ -22,6 +22,7 @@
*
* Authors:
*Rob Bradford
+ *Tiago Vignatti
*
*/
@@ -66,6 +67,12 @@ fill_bo(uint32_t handle, size_t size)
}
static void
+fill_bo_cpu(char *ptr)
+{
+ memcpy(ptr, pattern, sizeof(pattern));
+}
+
+static void
test_correct(void)
{
int dma_buf_fd;
@@ -180,6 +187,62 @@ test_forked(void)
gem_close(fd, handle);
}
+/* test CPU write. This has a rather big implication for the driver which must
+ * guarantee cache synchronization when writing the bo using CPU. */
+static void
+test_correct_cpu_write(void)
+{
+ int dma_buf_fd;
+ char *ptr;
+ uint32_t handle;
+
+ handle = gem_create(fd, BO_SIZE);
+
+ dma_buf_fd = prime_handle_to_fd(fd, handle);
+ igt_assert(errno == 0);
+
+ /* Check correctness of map using write protection (PROT_WRITE) */
+ ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE, MAP_SHARED,
dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+
+ /* Fill bo using CPU */
+ fill_bo_cpu(ptr);
+
+ /* Check pattern correctness */
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
+
+ munmap(ptr, BO_SIZE);
+ close(dma_buf_fd);
+ gem_close(fd, handle);
+}
+
+/* map from another process and then write using CPU */
+static void
+test_forked_cpu_write(void)
+{
+ int dma_buf_fd;
+ char *ptr;
+ uint32_t handle;
+
+ handle = gem_create(fd, BO_SIZE);
+
+ dma_buf_fd = prime_handle_to_fd(fd, handle);
+ igt_assert(errno == 0);
+
+ igt_fork(childno, 1) {
+ ptr = mmap(NULL, BO_SIZE, PROT_READ | PROT_WRITE , MAP_SHARED,
dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+ fill_bo_cpu(ptr);
+
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
+ munmap(ptr, BO_SIZE);
+ close(dma_buf_fd);
+ }
+ close(dma_buf_fd);
+ igt_waitchildren();
+ gem_close(fd, handle);
+}
+
static void
test_refcounting(void)
{
@@ -346,6 +409,8 @@ igt_main
{ "test_map_unmap", test_map_unmap },
{ "test_reprime", test_reprime },
{ "test_forked", test_forked },
+ { "test_correct_cpu_write", test_correct_cpu_write },
+ { "test_forked_cpu_write", test_forked_cpu_write },
{ "test_refcounting", test_refcounting },
{ "test_dup", test_dup },
{ "test_errors", test_errors },
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 4/4] drm/i915: Use CPU mapping for userspace dma-buf mmap()
Userspace is the one in charge of flush CPU by wrapping mmap with
begin{,end}_cpu_access.
v2: Remove LLC check cause we have dma-buf sync providers now. Also, fix return
before transferring ownership when mmap fails.
v3: Fix return values.
Signed-off-by: Tiago Vignatti
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index 8447ba4..8b87c86 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -193,7 +193,23 @@ static void i915_gem_dmabuf_kunmap(struct dma_buf
*dma_buf, unsigned long page_n
static int i915_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct
*vma)
{
- return -EINVAL;
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ int ret;
+
+ if (obj->base.size < vma->vm_end - vma->vm_start)
+ return -EINVAL;
+
+ if (WARN_ON(!obj->base.filp))
+ return -ENODEV;
+
+ ret = obj->base.filp->f_op->mmap(obj->base.filp, vma);
+ if (ret)
+ return ret;
+
+ fput(vma->vm_file);
+ vma->vm_file = get_file(obj->base.filp);
+
+ return 0;
}
static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, size_t start,
size_t length, enum dma_data_direction direction)
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t 2/5] prime_mmap: Fix a few misc stuff
- Remove pattern_check(), which was walking through a useless iterator
- Remove superfluous PROT_WRITE from gem_mmap, in test_correct()
- Add binary file to .gitignore
Signed-off-by: Tiago Vignatti
---
tests/.gitignore | 1 +
tests/prime_mmap.c | 37 -
2 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/tests/.gitignore b/tests/.gitignore
index 0af0899..5bc4a58 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -163,6 +163,7 @@ pm_sseu
prime_nv_api
prime_nv_pcopy
prime_nv_test
+prime_mmap
prime_self_import
prime_udl
template
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
index 4dc2055..dc59e8f 100644
--- a/tests/prime_mmap.c
+++ b/tests/prime_mmap.c
@@ -65,19 +65,6 @@ fill_bo(uint32_t handle, size_t size)
}
}
-static int
-pattern_check(char *ptr, size_t size)
-{
- off_t i;
- for (i = 0; i < size; i+=sizeof(pattern))
- {
- if (memcmp(ptr, pattern, sizeof(pattern)) != 0)
- return 1;
- }
-
- return 0;
-}
-
static void
test_correct(void)
{
@@ -92,14 +79,14 @@ test_correct(void)
igt_assert(errno == 0);
/* Check correctness vs GEM_MMAP_GTT */
- ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ | PROT_WRITE);
+ ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ);
ptr2 = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr1 != MAP_FAILED);
igt_assert(ptr2 != MAP_FAILED);
igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
/* Check pattern correctness */
- igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr2, pattern, sizeof(pattern)) == 0);
munmap(ptr1, BO_SIZE);
munmap(ptr2, BO_SIZE);
@@ -122,13 +109,13 @@ test_map_unmap(void)
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
/* Unmap and remap */
munmap(ptr, BO_SIZE);
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
munmap(ptr, BO_SIZE);
close(dma_buf_fd);
@@ -151,16 +138,16 @@ test_reprime(void)
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
close (dma_buf_fd);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
munmap(ptr, BO_SIZE);
dma_buf_fd = prime_handle_to_fd(fd, handle);
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
munmap(ptr, BO_SIZE);
close(dma_buf_fd);
@@ -184,7 +171,7 @@ test_forked(void)
igt_fork(childno, 1) {
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
munmap(ptr, BO_SIZE);
close(dma_buf_fd);
}
@@ -210,7 +197,7 @@ test_refcounting(void)
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
munmap(ptr, BO_SIZE);
close (dma_buf_fd);
}
@@ -231,7 +218,7 @@ test_dup(void)
ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
igt_assert(ptr != MAP_FAILED);
- igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr, pattern, sizeof(pattern)) == 0);
munmap(ptr, BO_SIZE);
gem_close(fd, handle);
close (dma_buf_fd);
@@ -310,7 +297,7 @@ test_aperture_limit(void)
igt_assert(errno == 0);
ptr1 = mmap(NULL, size1, PROT_READ, MAP_SHARED, dma_buf_fd1, 0);
igt_assert(ptr1 != MAP_FAILED);
- igt_assert(pattern_check(ptr1, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr1, pattern, sizeof(pattern)) == 0);
handle2 = gem_create(fd, size1);
fill_bo(handle2, BO_SIZE);
@@ -318,7 +305,7 @@ test_aperture_limit(void)
igt_assert(errno == 0);
ptr2 = mmap(NULL, size2, PROT_READ, MAP_SHARED, dma_buf_fd2, 0);
igt_assert(ptr2 != MAP_FAILED);
- igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+ igt_assert(memcmp(ptr2, pattern, sizeof(patte
[Intel-gfx] [PATCH i-g-t 1/5] prime_mmap: Add new test for calling mmap() on dma-buf fds
From: Rob Bradford
This test has the following subtests:
- test_correct for correctness of the data
- test_map_unmap checks for mapping idempotency
- test_reprime checks for dma-buf creation idempotency
- test_forked checks for multiprocess access
- test_refcounting checks for buffer reference counting
- test_dup chats that dup()ing the fd works
- test_errors checks the error return values for failures
- test_aperture_limit tests multiple buffer creation at the gtt aperture
limit
Signed-off-by: Rob Bradford
Signed-off-by: Tiago Vignatti
---
tests/Makefile.sources | 1 +
tests/prime_mmap.c | 383 +
2 files changed, 384 insertions(+)
create mode 100644 tests/prime_mmap.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c94714b..5b2072e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -90,6 +90,7 @@ TESTS_progs_M = \
pm_rps \
pm_rc6_residency \
pm_sseu \
+ prime_mmap \
prime_self_import \
template \
$(NULL)
diff --git a/tests/prime_mmap.c b/tests/prime_mmap.c
new file mode 100644
index 000..4dc2055
--- /dev/null
+++ b/tests/prime_mmap.c
@@ -0,0 +1,383 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *Rob Bradford
+ *
+ */
+
+/*
+ * Testcase: Check whether mmap()ing dma-buf works
+ */
+#define _GNU_SOURCE
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "drm.h"
+#include "i915_drm.h"
+#include "drmtest.h"
+#include "igt_debugfs.h"
+#include "ioctl_wrappers.h"
+
+#define BO_SIZE (16*1024)
+
+static int fd;
+
+char pattern[] = {0xff, 0x00, 0x00, 0x00,
+ 0x00, 0xff, 0x00, 0x00,
+ 0x00, 0x00, 0xff, 0x00,
+ 0x00, 0x00, 0x00, 0xff};
+
+static void
+fill_bo(uint32_t handle, size_t size)
+{
+ off_t i;
+ for (i = 0; i < size; i+=sizeof(pattern))
+ {
+ gem_write(fd, handle, i, pattern, sizeof(pattern));
+ }
+}
+
+static int
+pattern_check(char *ptr, size_t size)
+{
+ off_t i;
+ for (i = 0; i < size; i+=sizeof(pattern))
+ {
+ if (memcmp(ptr, pattern, sizeof(pattern)) != 0)
+ return 1;
+ }
+
+ return 0;
+}
+
+static void
+test_correct(void)
+{
+ int dma_buf_fd;
+ char *ptr1, *ptr2;
+ uint32_t handle;
+
+ handle = gem_create(fd, BO_SIZE);
+ fill_bo(handle, BO_SIZE);
+
+ dma_buf_fd = prime_handle_to_fd(fd, handle);
+ igt_assert(errno == 0);
+
+ /* Check correctness vs GEM_MMAP_GTT */
+ ptr1 = gem_mmap(fd, handle, BO_SIZE, PROT_READ | PROT_WRITE);
+ ptr2 = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+ igt_assert(ptr1 != MAP_FAILED);
+ igt_assert(ptr2 != MAP_FAILED);
+ igt_assert(memcmp(ptr1, ptr2, BO_SIZE) == 0);
+
+ /* Check pattern correctness */
+ igt_assert(pattern_check(ptr2, BO_SIZE) == 0);
+
+ munmap(ptr1, BO_SIZE);
+ munmap(ptr2, BO_SIZE);
+ close(dma_buf_fd);
+ gem_close(fd, handle);
+}
+
+static void
+test_map_unmap(void)
+{
+ int dma_buf_fd;
+ char *ptr;
+ uint32_t handle;
+
+ handle = gem_create(fd, BO_SIZE);
+ fill_bo(handle, BO_SIZE);
+
+ dma_buf_fd = prime_handle_to_fd(fd, handle);
+ igt_assert(errno == 0);
+
+ ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+ igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+ /* Unmap and remap */
+ munmap(ptr, BO_SIZE);
+ ptr = mmap(NULL, BO_SIZE, PROT_READ, MAP_SHARED, dma_buf_fd, 0);
+ igt_assert(ptr != MAP_FAILED);
+ igt_assert(pattern_check(ptr, BO_SIZE) == 0);
+
+ munmap(ptr, BO_SIZE);
+ close(dma_buf_fd);
+
[Intel-gfx] [PATCH 3/4] drm/i915: Implement end_cpu_access
Signed-off-by: Tiago Vignatti
---
drivers/gpu/drm/i915/i915_gem_dmabuf.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
index e9c2bfd..8447ba4 100644
--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
@@ -212,6 +212,15 @@ static int i915_gem_begin_cpu_access(struct dma_buf
*dma_buf, size_t start, size
return ret;
}
+static void i915_gem_end_cpu_access(struct dma_buf *dma_buf, size_t start,
size_t length, enum dma_data_direction direction)
+{
+ struct drm_i915_gem_object *obj = dma_buf_to_obj(dma_buf);
+ bool write = (direction == DMA_BIDIRECTIONAL || direction ==
DMA_TO_DEVICE);
+
+ if (i915_gem_object_set_to_gtt_domain(obj, write))
+ DRM_ERROR("failed to set bo into the GTT\n");
+}
+
static const struct dma_buf_ops i915_dmabuf_ops = {
.map_dma_buf = i915_gem_map_dma_buf,
.unmap_dma_buf = i915_gem_unmap_dma_buf,
@@ -224,6 +233,7 @@ static const struct dma_buf_ops i915_dmabuf_ops = {
.vmap = i915_gem_dmabuf_vmap,
.vunmap = i915_gem_dmabuf_vunmap,
.begin_cpu_access = i915_gem_begin_cpu_access,
+ .end_cpu_access = i915_gem_end_cpu_access,
};
struct dma_buf *i915_gem_prime_export(struct drm_device *dev,
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/4] drm: prime: Honour O_RDWR during prime-handle-to-fd
From: Daniel Thompson
Currently DRM_IOCTL_PRIME_HANDLE_TO_FD rejects all flags except
(DRM|O)_CLOEXEC making it difficult (maybe impossible) for userspace
to mmap() the resulting dma-buf even when this is supported by the
DRM driver.
It is trivial to relax the restriction and permit read/write access.
This is safe because the flags are seldom touched by drm; mostly they
are passed verbatim to dma_buf calls.
v3 (Tiago): removed unused flags variable from drm_prime_handle_to_fd_ioctl.
Signed-off-by: Daniel Thompson
Signed-off-by: Tiago Vignatti
---
drivers/gpu/drm/drm_prime.c | 10 +++---
include/uapi/drm/drm.h | 1 +
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 27aa718..df6cdc7 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -329,7 +329,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =
{
* drm_gem_prime_export - helper library implementation of the export callback
* @dev: drm_device to export from
* @obj: GEM object to export
- * @flags: flags like DRM_CLOEXEC
+ * @flags: flags like DRM_CLOEXEC and DRM_RDWR
*
* This is the implementation of the gem_prime_export functions for GEM drivers
* using the PRIME helpers.
@@ -628,7 +628,6 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
struct drm_file *file_priv)
{
struct drm_prime_handle *args = data;
- uint32_t flags;
if (!drm_core_check_feature(dev, DRIVER_PRIME))
return -EINVAL;
@@ -637,14 +636,11 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev,
void *data,
return -ENOSYS;
/* check flags are valid */
- if (args->flags & ~DRM_CLOEXEC)
+ if (args->flags & ~(DRM_CLOEXEC | DRM_RDWR))
return -EINVAL;
- /* we only want to pass DRM_CLOEXEC which is == O_CLOEXEC */
- flags = args->flags & DRM_CLOEXEC;
-
return dev->driver->prime_handle_to_fd(dev, file_priv,
- args->handle, flags, &args->fd);
+ args->handle, args->flags, &args->fd);
}
int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data,
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index 3801584..ad8223e 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -668,6 +668,7 @@ struct drm_set_client_cap {
__u64 value;
};
+#define DRM_RDWR O_RDWR
#define DRM_CLOEXEC O_CLOEXEC
struct drm_prime_handle {
__u32 handle;
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/4] dma-buf: Add ioctls to allow userspace to flush
From: Daniel Vetter
FIXME: Update kerneldoc for begin/end to make it clear that those are
for mmap too.
Open: Do we need a special indication that the begin/end is from
userspace mmap and not from kernel mmap?
There's also the question already about kernel internal users - vmap
and kmap interfaces are much different ... We might need to add a
mapping enum to the begin/end dma-buf functions.
v2 (Tiago): Fix header file type names (u64 -> __u64)
Signed-off-by: Daniel Vetter
Signed-off-by: Tiago Vignatti
---
drivers/dma-buf/dma-buf.c| 47
include/uapi/linux/dma-buf.h | 39
2 files changed, 86 insertions(+)
create mode 100644 include/uapi/linux/dma-buf.h
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index 155c146..4820d61 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -34,6 +34,8 @@
#include
#include
+#include
+
static inline int is_dma_buf_file(struct file *);
struct dma_buf_list {
@@ -251,11 +253,56 @@ out:
return events;
}
+static long dma_buf_ioctl(struct file *file,
+ unsigned int cmd, unsigned long arg)
+{
+ struct dma_buf *dmabuf;
+ struct dma_buf_sync sync;
+ enum dma_data_direction direction;
+
+ dmabuf = file->private_data;
+
+ if (!is_dma_buf_file(file))
+ return -EINVAL;
+
+ switch (cmd) {
+ case DMA_BUF_IOCTL_SYNC:
+ if (copy_from_user(&sync, (void __user *) arg, sizeof(sync)))
+ return -EFAULT;
+
+ if (sync.flags & DMA_BUF_SYNC_RW)
+ direction = DMA_BIDIRECTIONAL;
+ else if (sync.flags & DMA_BUF_SYNC_READ)
+ direction = DMA_FROM_DEVICE;
+ else if (sync.flags & DMA_BUF_SYNC_WRITE)
+ direction = DMA_TO_DEVICE;
+ else
+ return -EINVAL;
+
+ if (sync.flags & ~DMA_BUF_SYNC_VALID_FLAGS_MASK)
+ return -EINVAL;
+
+ /* FIXME: Check for overflows in start/length. */
+
+ if (sync.flags & DMA_BUF_SYNC_END)
+ dma_buf_end_cpu_access(dmabuf, sync.start,
+ sync.length, direction);
+ else
+ dma_buf_begin_cpu_access(dmabuf, sync.start,
+sync.length, direction);
+
+ return 0;
+ default:
+ return -ENOTTY;
+ }
+}
+
static const struct file_operations dma_buf_fops = {
.release= dma_buf_release,
.mmap = dma_buf_mmap_internal,
.llseek = dma_buf_llseek,
.poll = dma_buf_poll,
+ .unlocked_ioctl = dma_buf_ioctl,
};
/*
diff --git a/include/uapi/linux/dma-buf.h b/include/uapi/linux/dma-buf.h
new file mode 100644
index 000..e5327df
--- /dev/null
+++ b/include/uapi/linux/dma-buf.h
@@ -0,0 +1,39 @@
+/*
+ * Framework for buffer objects that can be shared across devices/subsystems.
+ *
+ * Copyright(C) 2015 Intel Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see
Re: [Intel-gfx] [PATCH v1 0/2] Enable legacy behaviour for Push constants
On 11.08.2015 17:44, Arun Siluvery wrote: > Patch1 fixes a simple compile error in Patch2 > Patch2 fixes gpu hang observed with a subtest of gem_concurrent_blit. > > Arun Siluvery (1): > drm/i915/gen9: Disable gather at set shader bit > > Mika Kuoppala (1): > drm/i915: Contain the WA_REG macro > > drivers/gpu/drm/i915/i915_reg.h | 5 + > drivers/gpu/drm/i915/intel_ringbuffer.c | 14 -- > 2 files changed, 17 insertions(+), 2 deletions(-) > prw-blt-overwrite-source-read-rcs-forked runs fine with these, tested on SKL-Y & -H Tested-by: Timo Aaltonen -- t ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: eDP can be present on DDI-E
On Tue, 2015-08-11 at 11:47 +0200, Daniel Vetter wrote:
> On Thu, Aug 06, 2015 at 03:51:39PM +0800, Xiong Zhang wrote:
> > From: Rodrigo Vivi
> >
> > On Skylake we have eDP-to-VGA using DDI-E and another aux.
> > So let's identify it properly.
>
> eDP means panel (the only difference in the code we have between eDP
> and
> DP is the power panel sequncing). VGA very much means no panel.
>
> Is this some impressive hack (dp->vga dongle using panel power as
> it's
> power source) or what's going on here? Or just confused commit
> message?
That's a good question. I've heard from customer the embedded converter
is eDP-to-VGA, not DP-to-VGA so I'm not sure what is behind and I have
no machine here with me.
Xiong, could you please check with customer if everything works without
this patch?
Thanks,
Rodrigo.
> I'll punt on this for now.
> -Daniel
>
> >
> > Also let's remove duplicated definitions to avoid later
> > confusion.
> >
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/intel_bios.h | 5 -
> > drivers/gpu/drm/i915/intel_dp.c | 9 +
> > 2 files changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.h
> > b/drivers/gpu/drm/i915/intel_bios.h
> > index 02255d8..a2ef0df 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.h
> > +++ b/drivers/gpu/drm/i915/intel_bios.h
> > @@ -747,11 +747,6 @@ int intel_parse_bios(struct drm_device *dev);
> > #defineDVO_C 2
> > #defineDVO_D 3
> >
> > -/* define the PORT for DP output type */
> > -#definePORT_IDPB 7
> > -#definePORT_IDPC 8
> > -#definePORT_IDPD 9
> > -
> > /* Possible values for the "DVO Port" field for versions >= 155:
> > */
> > #define DVO_PORT_HDMIA 0
> > #define DVO_PORT_HDMIB 1
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 7cd47bc..0643a91 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4978,16 +4978,17 @@ intel_trans_dp_port_sel(struct drm_crtc
> > *crtc)
> > return -1;
> > }
> >
> > -/* check the VBT to see whether the eDP is on DP-D port */
> > +/* check the VBT to see whether the eDP is on another port */
> > bool intel_dp_is_edp(struct drm_device *dev, enum port port)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > union child_device_config *p_child;
> > int i;
> > static const short port_mapping[] = {
> > - [PORT_B] = PORT_IDPB,
> > - [PORT_C] = PORT_IDPC,
> > - [PORT_D] = PORT_IDPD,
> > + [PORT_B] = DVO_PORT_DPB,
> > + [PORT_C] = DVO_PORT_DPC,
> > + [PORT_D] = DVO_PORT_DPD,
> > + [PORT_E] = DVO_PORT_DPE,
> > };
> >
> > if (port == PORT_A)
> > --
> > 2.1.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix stolen bios_reserved checks
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 7018 -Summary- Platform Delta drm-intel-nightly Series Applied ILK 302/302 302/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -1 283/283 282/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *BYT igt@gem_partial_pwrite_pread@reads-uncached PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.
On Tue, 2015-08-11 at 07:05 +, Zhang, Xiong Y wrote: > > -Original Message- > > From: Vivi, Rodrigo > > Sent: Saturday, August 8, 2015 8:34 AM > > To: intel-gfx@lists.freedesktop.org > > Cc: Vivi, Rodrigo; Zhang, Xiong Y > > Subject: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes. > > > > DDI-A and DDI-E shares the 4 lanes. So when DDI-E is present we > > need to > > configure lane count propperly for both. > > > > This was based on Sonika's > > [PATCH] drm/i915/skl: Select DDIA lane capability based upon vbt > > > > Credits-to: Sonika Jindal > > Cc: Xiong Zhang > > Signed-off-by: Rodrigo Vivi > > --- > > drivers/gpu/drm/i915/intel_ddi.c | 12 ++-- > > drivers/gpu/drm/i915/intel_dp.c | 8 +--- > > 2 files changed, 15 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index 110d546..557cecf 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -3178,7 +3178,15 @@ void intel_ddi_init(struct drm_device *dev, > > enum > > port port) > > struct intel_digital_port *intel_dig_port; > > struct intel_encoder *intel_encoder; > > struct drm_encoder *encoder; > > - bool init_hdmi, init_dp; > > + bool init_hdmi, init_dp, ddi_e_present; > > + > > + /* > > +* On SKL we don't have a way to detect DDI-E so we rely > > on VBT. > > +*/ > > + ddie_present = IS_SKYLAKE(dev) && > > + (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp > > || > > +dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi > > || > > +dev_priv > > ->vbt.ddi_port_info[PORT_E].supports_hdmi); > [Zhang, Xiong Y] ddie_present should be ddi_e_present > > > > init_hdmi = (dev_priv > > ->vbt.ddi_port_info[port].supports_dvi || > > dev_priv > > ->vbt.ddi_port_info[port].supports_hdmi); > > @@ -3210,7 +3218,7 @@ void intel_ddi_init(struct drm_device *dev, > > enum > > port port) > > intel_dig_port->port = port; > > intel_dig_port->saved_port_bits = > > I915_READ(DDI_BUF_CTL(port)) & > > (DDI_BUF_PORT_REVERSAL | > > - DDI_A_4_LANES); > > + ddi_e_present ? 0 : > > DDI_A_4_LANES); > [Zhang, Xiong Y] Sonika's patch will set DDI-A to 4 lanes when DDI-E > doesn't exist, > I think your patch will do nothing. Actually DDI_A_4_LANES is being already set unconditionally, so Sonika's patch has no effect. saved_port_bits goes to intel_dp->DP that goes to DDI_BUF_CTL and also it is used to calculate the number of lanes. With this patch we stop setting DDI_A_4_LANES when ddi_e is present so DDI-A keeps with 2 lanes and let other 2 lanes for DDI-E > > > > intel_encoder->type = INTEL_OUTPUT_UNKNOWN; > > intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); > > diff --git > > a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index > > 3ff2080..7ada79e 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -159,9 +159,11 @@ static u8 intel_dp_max_lane_count(struct > > intel_dp > > *intel_dp) > > u8 source_max, sink_max; > > > > source_max = 4; > > - if (HAS_DDI(dev) && intel_dig_port->port == PORT_A && > > - (intel_dig_port->saved_port_bits & DDI_A_4_LANES) == > > 0) > > - source_max = 2; > > + if (HAS_DDI(dev) && (intel_dig_port->port == PORT_E || > > +(intel_dig_port->port == PORT_A && > > + (intel_dig_port->saved_port_bits & > > + DDI_A_4_LANES) == 0)) > > + source_max = 2; > [Zhang, Xiong Y] it miss ')' at the end line. > > > > sink_max = drm_dp_max_lane_count(intel_dp->dpcd); > > > > -- > > 2.4.3 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/7] drm/i915: Don't use link_bw for PLL setup
From: Ville Syrjälä
Use port_clock instead of link_bw when picking the PLL parameters for
DP. link_bw may be zero with an eDP 1.4 sink that supports
DP_LINK_RATE_SET so we shouldn't use it for anything other than feed it
to the sink appropriately.
v2: Fix typo in commit message (Sivakumar)
Reviewed-by: Sivakumar Thulasimani
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/intel_ddi.c | 11 --
drivers/gpu/drm/i915/intel_dp.c | 44
2 files changed, 26 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 110d546..b183a3d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1574,17 +1574,14 @@ skl_ddi_pll_select(struct intel_crtc *intel_crtc,
DPLL_CFGCR2_PDIV(wrpll_params.pdiv) |
wrpll_params.central_freq;
} else if (intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT) {
- struct drm_encoder *encoder = &intel_encoder->base;
- struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-
- switch (intel_dp->link_bw) {
- case DP_LINK_BW_1_62:
+ switch (crtc_state->port_clock / 2) {
+ case 81000:
ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
0);
break;
- case DP_LINK_BW_2_7:
+ case 135000:
ctrl1 |=
DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_1350, 0);
break;
- case DP_LINK_BW_5_4:
+ case 27:
ctrl1 |=
DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_2700, 0);
break;
}
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fe5f4a2..3027c36 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -48,28 +48,28 @@
#define INTEL_DP_RESOLUTION_FAILSAFE (3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
struct dp_link_dpll {
- int link_bw;
+ int clock;
struct dpll dpll;
};
static const struct dp_link_dpll gen4_dpll[] = {
- { DP_LINK_BW_1_62,
+ { 162000,
{ .p1 = 2, .p2 = 10, .n = 2, .m1 = 23, .m2 = 8 } },
- { DP_LINK_BW_2_7,
+ { 27,
{ .p1 = 1, .p2 = 10, .n = 1, .m1 = 14, .m2 = 2 } }
};
static const struct dp_link_dpll pch_dpll[] = {
- { DP_LINK_BW_1_62,
+ { 162000,
{ .p1 = 2, .p2 = 10, .n = 1, .m1 = 12, .m2 = 9 } },
- { DP_LINK_BW_2_7,
+ { 27,
{ .p1 = 1, .p2 = 10, .n = 2, .m1 = 14, .m2 = 8 } }
};
static const struct dp_link_dpll vlv_dpll[] = {
- { DP_LINK_BW_1_62,
+ { 162000,
{ .p1 = 3, .p2 = 2, .n = 5, .m1 = 3, .m2 = 81 } },
- { DP_LINK_BW_2_7,
+ { 27,
{ .p1 = 2, .p2 = 2, .n = 1, .m1 = 2, .m2 = 27 } }
};
@@ -83,11 +83,11 @@ static const struct dp_link_dpll chv_dpll[] = {
* m2 is stored in fixed point format using formula below
* (m2_int << 22) | m2_fraction
*/
- { DP_LINK_BW_1_62, /* m2_int = 32, m2_fraction = 1677722 */
+ { 162000, /* m2_int = 32, m2_fraction = 1677722 */
{ .p1 = 4, .p2 = 2, .n = 1, .m1 = 2, .m2 = 0x81a } },
- { DP_LINK_BW_2_7, /* m2_int = 27, m2_fraction = 0 */
+ { 27, /* m2_int = 27, m2_fraction = 0 */
{ .p1 = 4, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } },
- { DP_LINK_BW_5_4, /* m2_int = 27, m2_fraction = 0 */
+ { 54, /* m2_int = 27, m2_fraction = 0 */
{ .p1 = 2, .p2 = 1, .n = 1, .m1 = 2, .m2 = 0x6c0 } }
};
@@ -1133,7 +1133,7 @@ intel_dp_connector_unregister(struct intel_connector
*intel_connector)
}
static void
-skl_edp_set_pll_config(struct intel_crtc_state *pipe_config, int link_clock)
+skl_edp_set_pll_config(struct intel_crtc_state *pipe_config)
{
u32 ctrl1;
@@ -1145,7 +1145,7 @@ skl_edp_set_pll_config(struct intel_crtc_state
*pipe_config, int link_clock)
pipe_config->dpll_hw_state.cfgcr2 = 0;
ctrl1 = DPLL_CTRL1_OVERRIDE(SKL_DPLL0);
- switch (link_clock / 2) {
+ switch (pipe_config->port_clock / 2) {
case 81000:
ctrl1 |= DPLL_CTRL1_LINK_RATE(DPLL_CTRL1_LINK_RATE_810,
SKL_DPLL0);
@@ -1179,19 +1179,19 @@ skl_edp_set_pll_config(struct intel_crtc_state
*pipe_config, int link_clock)
}
static void
-hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config, int link_bw)
+hsw_dp_set_ddi_pll_sel(struct intel_crtc_state *pipe_config)
{
memset(&pipe_config->dpll_hw_state, 0,
sizeof(pipe_config->dpll_hw_state));
- switch (link_bw) {
- case DP_LINK_BW_1_62:
+ switch (pipe_config->port_clock / 2) {
+ ca
Re: [Intel-gfx] [PATCH v2] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
On Tue, Aug 11, 2015 at 07:47:10PM +0300, ville.syrj...@linux.intel.com wrote: > From: Ville Syrjälä > > Currently we don't clflush on pin_to_display if the bo is already > UC/WT and is not in the CPU write domain. This causes problems with > pwrite since pwrite doesn't change the write domain, and it avoids > clflushing on UC/WT buffers on LLC platforms unless the buffer is > currently being scanned out. > > Fix the problem by marking the cache dirty and adjusting > i915_gem_object_set_cache_level() to clflush when the cache is dirty > even if the cache_level doesn't change. > > My last attempt [1] at fixing this via write domain frobbing was shot > down, but now with the cache_dirty flag we can do things in a nicer way. > > [1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html > > v2: Drop the I915_CACHE_NONE/WT checks from pwrite > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422 > Testcase: igt/kms_pwrite_crc > Testcase: igt/gem_pwrite_snooped > Signed-off-by: Ville Syrjälä Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] tests/gem_pwrite_snooped: Verify set_caching vs. pwrite clflush behaviour
From: Ville Syrjälä
The test does the following
1. set_domain src GTT
2. set_caching src NONE
3. pwrite src
4. set_caching src CACHED
5. blt src->dst
6. pread dst
7. verify data matches
Signed-off-by: Ville Syrjälä
---
tests/Makefile.sources | 1 +
tests/gem_pwrite_snooped.c | 140 +
2 files changed, 141 insertions(+)
create mode 100644 tests/gem_pwrite_snooped.c
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index c94714b..b9a4cb4 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -47,6 +47,7 @@ TESTS_progs_M = \
gem_pread_after_blit \
gem_pwrite \
gem_pwrite_pread \
+ gem_pwrite_snooped \
gem_readwrite \
gem_read_read_speed \
gem_reloc_overflow \
diff --git a/tests/gem_pwrite_snooped.c b/tests/gem_pwrite_snooped.c
new file mode 100644
index 000..890c61d
--- /dev/null
+++ b/tests/gem_pwrite_snooped.c
@@ -0,0 +1,140 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "drm.h"
+#include "ioctl_wrappers.h"
+#include "intel_chipset.h"
+#include "drmtest.h"
+#include "igt_aux.h"
+
+IGT_TEST_DESCRIPTION(
+ "pwrite to a snooped bo then make it uncached and check that the GPU sees
the data.");
+
+static int fd;
+static uint32_t devid;
+static drm_intel_bufmgr *bufmgr;
+
+static void blit(drm_intel_bo *dst, drm_intel_bo *src,
+unsigned int width, unsigned int height,
+unsigned int dst_pitch, unsigned int src_pitch)
+{
+ struct intel_batchbuffer *batch;
+
+ batch = intel_batchbuffer_alloc(bufmgr, devid);
+ igt_assert(batch);
+
+ BLIT_COPY_BATCH_START(0);
+ OUT_BATCH((3 << 24) | /* 32 bits */
+ (0xcc << 16) | /* copy ROP */
+ dst_pitch);
+ OUT_BATCH(0 << 16 | 0);
+ OUT_BATCH(height << 16 | width);
+ OUT_RELOC_FENCED(dst, I915_GEM_DOMAIN_RENDER, I915_GEM_DOMAIN_RENDER,
0);
+ OUT_BATCH(0 << 16 | 0);
+ OUT_BATCH(src_pitch);
+ OUT_RELOC_FENCED(src, I915_GEM_DOMAIN_RENDER, 0, 0);
+ ADVANCE_BATCH();
+
+ if (batch->gen >= 6) {
+ BEGIN_BATCH(3, 0);
+ OUT_BATCH(XY_SETUP_CLIP_BLT_CMD);
+ OUT_BATCH(0);
+ OUT_BATCH(0);
+ ADVANCE_BATCH();
+ }
+
+ intel_batchbuffer_flush(batch);
+ intel_batchbuffer_free(batch);
+}
+
+static void *memchr_inv(const void *s, int c, size_t n)
+{
+ const unsigned char *us = s;
+ unsigned char uc = c;
+
+ while (n--) {
+ if (*us != uc)
+ return (void *) us;
+ us++;
+ }
+
+ return NULL;
+}
+
+static void test(int w, int h)
+{
+ int object_size = w * h * 4;
+ drm_intel_bo *src, *dst;
+ void *buf;
+
+ src = drm_intel_bo_alloc(bufmgr, "src", object_size, 4096);
+ igt_assert(src);
+ dst = drm_intel_bo_alloc(bufmgr, "dst", object_size, 4096);
+ igt_assert(dst);
+
+ buf = malloc(object_size);
+ igt_assert(buf);
+ memset(buf, 0xff, object_size);
+
+ gem_set_domain(fd, src->handle, I915_GEM_DOMAIN_GTT,
+ I915_GEM_DOMAIN_GTT);
+
+ gem_set_caching(fd, src->handle, I915_CACHING_CACHED);
+
+ gem_write(fd, src->handle, 0, buf, object_size);
+
+ gem_set_caching(fd, src->handle, I915_CACHING_NONE);
+
+ blit(dst, src, w, h, w * 4, h * 4);
+
+ memset(buf, 0x00, object_size);
+ gem_read(fd, dst->handle, 0, buf, object_size);
+
+ igt_assert(memchr_inv(buf, 0xff, object_size) == NULL);
+}
+
+igt_simple_main
+{
+ igt_skip_on_simulation();
+
+ fd = drm_open_any();
+ devid = intel_get_drm_de
[Intel-gfx] [PATCH v2] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
From: Ville Syrjälä
Currently we don't clflush on pin_to_display if the bo is already
UC/WT and is not in the CPU write domain. This causes problems with
pwrite since pwrite doesn't change the write domain, and it avoids
clflushing on UC/WT buffers on LLC platforms unless the buffer is
currently being scanned out.
Fix the problem by marking the cache dirty and adjusting
i915_gem_object_set_cache_level() to clflush when the cache is dirty
even if the cache_level doesn't change.
My last attempt [1] at fixing this via write domain frobbing was shot
down, but now with the cache_dirty flag we can do things in a nicer way.
[1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
v2: Drop the I915_CACHE_NONE/WT checks from pwrite
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
Testcase: igt/kms_pwrite_crc
Testcase: igt/gem_pwrite_snooped
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_gem.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..f828dc7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1005,12 +1005,14 @@ out:
if (!needs_clflush_after &&
obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
if (i915_gem_clflush_object(obj, obj->pin_display))
- i915_gem_chipset_flush(dev);
+ needs_clflush_after = true;
}
}
if (needs_clflush_after)
i915_gem_chipset_flush(dev);
+ else
+ obj->cache_dirty = true;
intel_fb_obj_flush(obj, false, ORIGIN_CPU);
return ret;
@@ -3639,10 +3641,10 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct i915_vma *vma, *next;
- int ret;
+ int ret = 0;
if (obj->cache_level == cache_level)
- return 0;
+ goto out;
if (i915_gem_obj_is_pinned(obj)) {
DRM_DEBUG("can not change the cache level of pinned objects\n");
@@ -3687,6 +3689,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
vma->node.color = cache_level;
obj->cache_level = cache_level;
+out:
if (obj->cache_dirty &&
obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
cpu_write_needs_clflush(obj)) {
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 5/5] tests: make drm_read platform agnostic
On Tue, Aug 11, 2015 at 11:59:16AM -0400, Micah Fedke wrote: > > Update the drm_read test to operate on any platform to demonstrate the use of > drm_open_driver(OPEN_ANY_GPU). > > To work on exynos, the event generation code is converted to use the new CRTC > selection API for vblank. The first valid crtc is selected at fixture-time. Pardon? If exoynos doesn't implement the generic drmWaitVBlank(), fix it. > pipe0_enabled() is updated to use the drmMode* wrapper functions instead of > direct ioctls, and the unnecessary, intel-specific pipe<->crtc mapping ioctl > is > dropped. Just drop the pipe lookup, we can just indeed just use index 0. > With these updates in place, drm_read can run successfully on intel and > exynos. > Tested on ivb and peach-pi, respectively. You don't need to change that much, especially changing the path through the kernel. Nak. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 5/5] tests: make drm_read platform agnostic
Update the drm_read test to operate on any platform to demonstrate the use of
drm_open_driver(OPEN_ANY_GPU).
To work on exynos, the event generation code is converted to use the new CRTC
selection API for vblank. The first valid crtc is selected at fixture-time.
pipe0_enabled() is updated to use the drmMode* wrapper functions instead of
direct ioctls, and the unnecessary, intel-specific pipe<->crtc mapping ioctl is
dropped.
With these updates in place, drm_read can run successfully on intel and exynos.
Tested on ivb and peach-pi, respectively.
Signed-off-by: Micah Fedke
---
tests/drm_read.c | 87
1 file changed, 50 insertions(+), 37 deletions(-)
diff --git a/tests/drm_read.c b/tests/drm_read.c
index fdaf126..38fde26 100644
--- a/tests/drm_read.c
+++ b/tests/drm_read.c
@@ -45,10 +45,15 @@
#include "drm.h"
#include "ioctl_wrappers.h"
#include "drmtest.h"
+#include "igt_core.h"
#include "igt_aux.h"
+#include "igt_kms.h"
IGT_TEST_DESCRIPTION("Call read(drm) and see if it behaves.");
+static drmModeRes *resources;
+static int crtc_idx;
+
static void sighandler(int sig)
{
}
@@ -61,16 +66,19 @@ static void assert_empty(int fd)
static void generate_event(int fd)
{
- union drm_wait_vblank vbl;
+ drmVBlank wait_vbl;
+ unsigned crtc_idx_mask;
+ memset(&wait_vbl, 0, sizeof(wait_vbl));
- /* We require that pipe 0 is running */
+ crtc_idx_mask = crtc_idx << DRM_VBLANK_HIGH_CRTC_SHIFT;
+ igt_assert(!(crtc_idx_mask & ~DRM_VBLANK_HIGH_CRTC_MASK));
- vbl.request.type =
- DRM_VBLANK_RELATIVE |
- DRM_VBLANK_EVENT;
- vbl.request.sequence = 0;
+ wait_vbl.request.type = crtc_idx_mask;
+ wait_vbl.request.type |= DRM_VBLANK_RELATIVE;
+ wait_vbl.request.type |= DRM_VBLANK_EVENT;
+ wait_vbl.request.sequence = 1;
- do_ioctl(fd, DRM_IOCTL_WAIT_VBLANK, &vbl);
+ igt_assert(!drmWaitVBlank(fd, &wait_vbl));
}
static void wait_for_event(int fd)
@@ -154,44 +162,27 @@ static void test_short_buffer(int in, int nonblock)
static int pipe0_enabled(int fd)
{
- struct drm_mode_card_res res;
- uint32_t crtcs[32];
- int i;
+ drmModeRes *res;
+ drmModeCrtc *crtc;
+ int ret;
/* We assume we can generate events on pipe 0. So we have better
* make sure that is running!
*/
- memset(&res, 0, sizeof(res));
- res.count_crtcs = 32;
- res.crtc_id_ptr = (uintptr_t)crtcs;
-
- if (drmIoctl(fd, DRM_IOCTL_MODE_GETRESOURCES, &res))
- return 0;
-
- if (res.count_crtcs > 32)
+ res = drmModeGetResources(fd);
+ igt_assert(res);
+ crtc = drmModeGetCrtc(fd, res->crtcs[crtc_idx]);
+ if (!crtc){
return 0;
+ }
- for (i = 0; i < res.count_crtcs; i++) {
- struct drm_i915_get_pipe_from_crtc_id get_pipe;
- struct drm_mode_crtc mode;
-
- memset(&get_pipe, 0, sizeof(get_pipe));
- memset(&mode, 0, sizeof(mode));
-
- mode.crtc_id = crtcs[i];
-
- get_pipe.pipe = -1;
- get_pipe.crtc_id = mode.crtc_id;
- drmIoctl(fd, DRM_IOCTL_I915_GET_PIPE_FROM_CRTC_ID, &get_pipe);
- if (get_pipe.pipe)
- continue;
+ ret = crtc->mode_valid && crtc->mode.clock;
- drmIoctl(fd, DRM_IOCTL_MODE_GETCRTC, &mode);
- return mode.mode_valid && mode.mode.clock;
- }
+ drmModeFreeCrtc(crtc);
+ drmModeFreeResources(res);
- return 0;
+ return ret;
}
igt_main
@@ -202,8 +193,30 @@ igt_main
siginterrupt(SIGALRM, 1);
igt_fixture {
- fd = drm_open_driver_master(DRIVER_INTEL);
+ struct kmstest_connector_config config;
+ int i, n;
+
+ fd = drm_open_driver_master(OPEN_ANY_GPU);
+ igt_enable_connectors(fd);
+ kmstest_set_vt_graphics_mode();
+
igt_require(pipe0_enabled(fd));
+
+ resources = drmModeGetResources(fd);
+ igt_assert(resources);
+
+ for (i = 0; i < resources->count_connectors; i++) {
+ for (n = 0; n < resources->count_crtcs; n++) {
+ //use the first connector config we find
+ if(kmstest_get_connector_config(fd,
resources->connectors[i],
+ 1 << n, &config)){
+ crtc_idx = config.crtc_idx;
+ break;
+ }
+ }
+ }
+ drmModeFreeCrtc(config.crtc);
+
}
igt_subtest("invalid-buffer")
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedes
[Intel-gfx] [PATCH v2 4/5] lib/tests: make kmstest_get_pipe_from_crtc_id and igt_enable_connectors generic to prepare for platform agnostic tests
There is some miscellaneous plumbing to do if we are going to have a test that
runs on all platforms. intel and exynos are the current targets. The test we
are aiming to support is drm_read. There is much intel-specific code
throughout the support library, so anyone attempting to make other tests
platform agnostic will run into more of this type of thing.
kmstest_get_connector_config() issues an intel-specific ioctl via
kmstest_get_pipe_from_crtc_id() and asserts on the return. This patch removes
the call to kmstest_get_pipe_from_crtc_id altogether and instead assigns the
crct id according to its array index in the drmModeRes struct. This seems to
be the default behavior for the drm anyway.
igt_enable_connectors() reopens the drm dev node to perform its work, but it
can't know which chipset to specify to the new drm_open_driver*() API. This
patch modifies igt_enable_connectors() to take an already open drm fd to work
on, so that the chipset is implied. igt_enable_connectors() performs generic
drm work only, so it should not need to do anything chipset-specific. There
could be an argument here for isolating this work on a separate fd, but I can't
see how it would make a difference to the drm.
Signed-off-by: Micah Fedke
---
lib/igt_kms.c | 9 ++---
lib/igt_kms.h | 2 +-
tests/kms_flip.c | 2 +-
tests/kms_pipe_crc_basic.c | 2 +-
4 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7e956b4..78c98d2 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -629,8 +629,7 @@ found:
config->encoder = encoder;
config->crtc = drmModeGetCrtc(drm_fd, resources->crtcs[i]);
config->crtc_idx = i;
- config->pipe = kmstest_get_pipe_from_crtc_id(drm_fd,
-config->crtc->crtc_id);
+ config->pipe = i;
drmModeFreeResources(resources);
@@ -1801,13 +1800,10 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe)
* An exit handler is installed to ensure connectors are reset when the test
* exits.
*/
-void igt_enable_connectors(void)
+void igt_enable_connectors(int drm_fd)
{
drmModeRes *res;
drmModeConnector *c;
- int drm_fd;
-
- drm_fd = drm_open_driver(DRIVER_INTEL);
res = drmModeGetResources(drm_fd);
@@ -1830,7 +1826,6 @@ void igt_enable_connectors(void)
drmModeFreeConnector(c);
}
- close(drm_fd);
}
/**
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 565df14..b5f007d 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -270,7 +270,7 @@ void igt_wait_for_vblank(int drm_fd, enum pipe pipe);
#define IGT_FIXED(i,f) ((i) << 16 | (f))
-void igt_enable_connectors(void);
+void igt_enable_connectors(int drm_fd);
void igt_reset_connectors(void);
#define EDID_LENGTH 128
diff --git a/tests/kms_flip.c b/tests/kms_flip.c
index 4d8d0c0..a88e707 100644
--- a/tests/kms_flip.c
+++ b/tests/kms_flip.c
@@ -1702,7 +1702,7 @@ int main(int argc, char **argv)
igt_fixture {
drm_fd = drm_open_driver_master(DRIVER_INTEL);
- igt_enable_connectors();
+ igt_enable_connectors(drm_fd);
kmstest_set_vt_graphics_mode();
igt_install_exit_handler(kms_flip_exit_handler);
diff --git a/tests/kms_pipe_crc_basic.c b/tests/kms_pipe_crc_basic.c
index fc4ad46..10ef097 100644
--- a/tests/kms_pipe_crc_basic.c
+++ b/tests/kms_pipe_crc_basic.c
@@ -228,7 +228,7 @@ igt_main
igt_fixture {
data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
- igt_enable_connectors();
+ igt_enable_connectors(data.drm_fd);
kmstest_set_vt_graphics_mode();
--
2.5.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 3/5] lib: remove support for deprecated drm_open_any*() calls
Signed-off-by: Micah Fedke --- lib/drmtest.h | 6 -- 1 file changed, 6 deletions(-) diff --git a/lib/drmtest.h b/lib/drmtest.h index dcb0c34..41ddbe7 100644 --- a/lib/drmtest.h +++ b/lib/drmtest.h @@ -41,12 +41,6 @@ #define OPEN_ANY_GPU 0x1 #define DRIVER_INTEL (0x1 << 1) -/* provide the deprecated drm_open_any*() calls */ -#define drm_open_any() drm_open_driver(OPEN_ANY_GPU) -#define drm_open_any_master() drm_open_driver_master(OPEN_ANY_GPU) -#define drm_open_any_render() drm_open_driver_render(OPEN_ANY_GPU) -#define __drm_open_any() __drm_open_driver(OPEN_ANY_GPU) - #ifdef ANDROID #ifndef HAVE_MMAP64 -- 2.5.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v2 2/5] convert drm_open_any*() calls to drm_open_driver*(DRIVER_INTEL) calls with cocci
Apply the new API to all call sites within the test suite using the following semantic patch: // Semantic patch for replacing drm_open_any* with arch-specific drm_open_driver* calls @@ identifier i =~ "\bdrm_open_any\b"; @@ - i() + drm_open_driver(DRIVER_INTEL) @@ identifier i =~ "\bdrm_open_any_master\b"; @@ - i() + drm_open_driver_master(DRIVER_INTEL) @@ identifier i =~ "\bdrm_open_any_render\b"; @@ - i() + drm_open_driver_render(DRIVER_INTEL) @@ identifier i =~ "\b__drm_open_any\b"; @@ - i() + __drm_open_driver(DRIVER_INTEL) Signed-off-by: Micah Fedke --- benchmarks/gem_userptr_benchmark.c | 2 +- benchmarks/intel_upload_blit_large.c | 2 +- benchmarks/intel_upload_blit_large_gtt.c | 2 +- benchmarks/intel_upload_blit_large_map.c | 2 +- benchmarks/intel_upload_blit_small.c | 2 +- debugger/eudb.c | 2 +- lib/igt_gt.c | 2 +- lib/igt_kms.c| 2 +- tests/core_get_client_auth.c | 6 ++--- tests/core_getclient.c | 2 +- tests/core_getstats.c| 2 +- tests/core_getversion.c | 2 +- tests/drm_import_export.c| 4 ++-- tests/drm_read.c | 2 +- tests/drm_vma_limiter.c | 2 +- tests/drm_vma_limiter_cached.c | 2 +- tests/drm_vma_limiter_cpu.c | 2 +- tests/drm_vma_limiter_gtt.c | 2 +- tests/drv_getparams.c| 2 +- tests/drv_hangman.c | 4 ++-- tests/drv_suspend.c | 2 +- tests/eviction_common.c | 2 +- tests/gem_alive.c| 2 +- tests/gem_bad_address.c | 2 +- tests/gem_bad_batch.c| 2 +- tests/gem_bad_blit.c | 2 +- tests/gem_bad_length.c | 2 +- tests/gem_bad_reloc.c| 2 +- tests/gem_basic.c| 2 +- tests/gem_caching.c | 2 +- tests/gem_concurrent_blit.c | 4 ++-- tests/gem_cpu_reloc.c| 2 +- tests/gem_cs_prefetch.c | 2 +- tests/gem_cs_tlb.c | 2 +- tests/gem_ctx_bad_destroy.c | 2 +- tests/gem_ctx_bad_exec.c | 2 +- tests/gem_ctx_basic.c| 4 ++-- tests/gem_ctx_create.c | 2 +- tests/gem_ctx_exec.c | 2 +- tests/gem_ctx_param_basic.c | 2 +- tests/gem_ctx_thrash.c | 4 ++-- tests/gem_double_irq_loop.c | 2 +- tests/gem_dummy_reloc_loop.c | 4 ++-- tests/gem_evict_alignment.c | 2 +- tests/gem_evict_everything.c | 2 +- tests/gem_exec_bad_domains.c | 2 +- tests/gem_exec_big.c | 2 +- tests/gem_exec_blt.c | 2 +- tests/gem_exec_faulting_reloc.c | 2 +- tests/gem_exec_lut_handle.c | 2 +- tests/gem_exec_nop.c | 2 +- tests/gem_exec_params.c | 2 +- tests/gem_exec_parse.c | 2 +- tests/gem_fd_exhaustion.c| 2 +- tests/gem_fence_thrash.c | 2 +- tests/gem_fence_upload.c | 8 +++ tests/gem_fenced_exec_thrash.c | 2 +- tests/gem_flink.c| 6 ++--- tests/gem_flink_race.c | 6 ++--- tests/gem_gpgpu_fill.c | 2 +- tests/gem_gtt_cpu_tlb.c | 2 +- tests/gem_gtt_hog.c | 4 ++-- tests/gem_gtt_speed.c| 2 +- tests/gem_hang.c | 2 +- tests/gem_hangcheck_forcewake.c | 2 +- tests/gem_largeobject.c | 2 +- tests/gem_linear_blits.c | 2 +- tests/gem_lut_handle.c | 2 +- tests/gem_madvise.c | 8 +++ tests/gem_media_fill.c | 2 +- tests/gem_mmap.c | 2 +- tests/gem_mmap_gtt.c | 4 ++-- tests/gem_mmap_offset_exhaustion.c | 2 +- tests/gem_mmap_wc.c | 2 +- tests/gem_multi_bsd_sync_loop.c | 4 ++-- tests/gem_non_secure_batch.c | 2 +- tests/gem_partial_pwrite_pread.c | 2 +- tests/gem_persistent_relocs.c| 2 +- tests/gem_pin.c | 2 +- tests/gem_pipe_control_store_loop.c | 2 +- tests/gem_ppgtt.c| 4 ++-- tests/gem_pread.c| 2 +- tests/gem_pread_after_blit.c | 2 +- tests/gem_pwrite.c | 2 +- tests/gem_pwrite_pread.c | 2 +- tests/gem_readwrite.c| 2 +- tests/gem_reg_read.c | 2 +- tests/gem_reloc_overflow.c
[Intel-gfx] [PATCH v2 0/5] igt: adding parameter to drm_open_any and drm_open_any_master to allow specification of non-intel GPUs
Changes since last version of patch: Addressed comments from danvet and Chris Wilson: - removed api doc for __get_drm_device_name - included coccinelle script with commit - added signed-off-by and detailed comments to commit messages, including justification of drm_read changes - updated coding style and spaces-and-braces to match kernel coding style (let me know if i missed anything) - simplified crtc<->pipe mapping code by replacing call to kmstest_get_pipe_from_crtc_id() with array index Micah Fedke (5): lib: adding drm_open_driver() interface convert drm_open_any*() calls to drm_open_driver*(DRIVER_INTEL) calls with cocci lib: remove support for deprecated drm_open_any*() calls lib/tests: make kmstest_get_pipe_from_crtc_id and igt_enable_connectors generic to prepare for platform agnostic tests tests: make drm_read platform agnostic benchmarks/gem_userptr_benchmark.c | 2 +- benchmarks/intel_upload_blit_large.c | 2 +- benchmarks/intel_upload_blit_large_gtt.c | 2 +- benchmarks/intel_upload_blit_large_map.c | 2 +- benchmarks/intel_upload_blit_small.c | 2 +- debugger/eudb.c | 2 +- lib/drmtest.c| 97 lib/drmtest.h| 12 ++-- lib/igt_gt.c | 2 +- lib/igt_kms.c| 9 +-- lib/igt_kms.h| 2 +- tests/core_get_client_auth.c | 6 +- tests/core_getclient.c | 2 +- tests/core_getstats.c| 2 +- tests/core_getversion.c | 2 +- tests/drm_import_export.c| 4 +- tests/drm_read.c | 87 tests/drm_vma_limiter.c | 2 +- tests/drm_vma_limiter_cached.c | 2 +- tests/drm_vma_limiter_cpu.c | 2 +- tests/drm_vma_limiter_gtt.c | 2 +- tests/drv_getparams.c| 2 +- tests/drv_hangman.c | 4 +- tests/drv_suspend.c | 2 +- tests/eviction_common.c | 2 +- tests/gem_alive.c| 2 +- tests/gem_bad_address.c | 2 +- tests/gem_bad_batch.c| 2 +- tests/gem_bad_blit.c | 2 +- tests/gem_bad_length.c | 2 +- tests/gem_bad_reloc.c| 2 +- tests/gem_basic.c| 2 +- tests/gem_caching.c | 2 +- tests/gem_concurrent_blit.c | 4 +- tests/gem_cpu_reloc.c| 2 +- tests/gem_cs_prefetch.c | 2 +- tests/gem_cs_tlb.c | 2 +- tests/gem_ctx_bad_destroy.c | 2 +- tests/gem_ctx_bad_exec.c | 2 +- tests/gem_ctx_basic.c| 4 +- tests/gem_ctx_create.c | 2 +- tests/gem_ctx_exec.c | 2 +- tests/gem_ctx_param_basic.c | 2 +- tests/gem_ctx_thrash.c | 4 +- tests/gem_double_irq_loop.c | 2 +- tests/gem_dummy_reloc_loop.c | 4 +- tests/gem_evict_alignment.c | 2 +- tests/gem_evict_everything.c | 2 +- tests/gem_exec_bad_domains.c | 2 +- tests/gem_exec_big.c | 2 +- tests/gem_exec_blt.c | 2 +- tests/gem_exec_faulting_reloc.c | 2 +- tests/gem_exec_lut_handle.c | 2 +- tests/gem_exec_nop.c | 2 +- tests/gem_exec_params.c | 2 +- tests/gem_exec_parse.c | 2 +- tests/gem_fd_exhaustion.c| 2 +- tests/gem_fence_thrash.c | 2 +- tests/gem_fence_upload.c | 8 +-- tests/gem_fenced_exec_thrash.c | 2 +- tests/gem_flink.c| 6 +- tests/gem_flink_race.c | 6 +- tests/gem_gpgpu_fill.c | 2 +- tests/gem_gtt_cpu_tlb.c | 2 +- tests/gem_gtt_hog.c | 4 +- tests/gem_gtt_speed.c| 2 +- tests/gem_hang.c | 2 +- tests/gem_hangcheck_forcewake.c | 2 +- tests/gem_largeobject.c | 2 +- tests/gem_linear_blits.c | 2 +- tests/gem_lut_handle.c | 2 +- tests/gem_madvise.c | 8 +-- tests/gem_media_fill.c | 2 +- tests/gem_mmap.c | 2 +- tests/gem_mmap_gtt.c | 4 +- tests/gem_mmap_offset_exhaustion.c | 2 +- tests/gem_mmap_wc.c | 2 +- tests/gem_multi_bsd_sync_loop.c | 4 +- tests/gem_non_secure_batch.c | 2 +- tests/gem_partial_pwrite_pread.c | 2 +- tests/gem_persistent_relocs.c| 2 +- tests/ge
[Intel-gfx] [PATCH v2 1/5] lib: adding drm_open_driver() interface
The drm_open_driver*() functions replace the drm_open_any*() functions and
provide the same utility, but in a way that is platform agnostic, not
intel-specific. This opens the path for adopting intel-gpu-tools to non-intel
platforms.
This commit renames the calls and adds the chipset parameter which can be used
to restrict the opening to a specific hardware family. For example,
drm_open_driver(DRIVER_INTEL) will only return a valid fd if an intel GPU is
found on the system, along with performing intel-specific initialization stuff
like gem_quiescent_gpu(), et al. If OPEN_ANY_GPU is specified, the first
available drm device of any type will be opened.
Other hardware type flags may be added in the future.
The drm_open_any*() calls are retained as aliases of
drm_open_driver*(OPEN_ANY_GPU) but will be removed in a subsequent patch.
Signed-off-by: Micah Fedke
---
lib/drmtest.c | 97 +--
lib/drmtest.h | 18 ---
2 files changed, 75 insertions(+), 40 deletions(-)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index ee5c123..44a0f62 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -75,23 +75,32 @@
uint16_t __drm_device_id;
-static int is_i915_device(int fd)
+static int __get_drm_device_name(int fd, char *name)
{
drm_version_t version;
- char name[5] = "";
memset(&version, 0, sizeof(version));
version.name_len = 4;
version.name = name;
- if (drmIoctl(fd, DRM_IOCTL_VERSION, &version))
+ if (!drmIoctl(fd, DRM_IOCTL_VERSION, &version)){
return 0;
+ }
+
+ return -1;
+}
- return strcmp("i915", name) == 0;
+static bool is_i915_device(int fd)
+{
+ int ret;
+ char name[5] = "";
+
+ ret = __get_drm_device_name(fd, name);
+
+ return !ret && strcmp("i915", name) == 0;
}
-static int
-is_intel(int fd)
+static bool is_intel(int fd)
{
struct drm_i915_getparam gp;
int devid = 0;
@@ -101,13 +110,13 @@ is_intel(int fd)
gp.value = &devid;
if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp, sizeof(gp)))
- return 0;
+ return false;
if (!IS_INTEL(devid))
- return 0;
+ return false;
__drm_device_id = devid;
- return 1;
+ return true;
}
static void check_stop_rings(void)
@@ -230,19 +239,31 @@ int drm_get_card(void)
return -1;
}
-/** Open the first DRM device we can find, searching up to 16 device nodes */
-int __drm_open_any(void)
+/**
+ * __drm_open_driver:
+ *
+ * Open the first DRM device we can find, searching up to 16 device nodes
+ *
+ * @chipset: OR'd flags for each chipset to search, eg. DRIVER_INTEL
+ *
+ * Returns:
+ * An open DRM fd or -1 on error
+ */
+int __drm_open_driver(int chipset)
{
for (int i = 0; i < 16; i++) {
char name[80];
int fd;
+ bool found_intel;
sprintf(name, "/dev/dri/card%u", i);
fd = open(name, O_RDWR);
if (fd == -1)
continue;
- if (is_i915_device(fd) && is_intel(fd))
+ found_intel = is_i915_device(fd) && is_intel(fd) && (chipset &
DRIVER_INTEL);
+
+ if ((chipset & OPEN_ANY_GPU) || found_intel)
return fd;
close(fd);
@@ -252,7 +273,7 @@ int __drm_open_any(void)
return -1;
}
-static int __drm_open_any_render(void)
+static int __drm_open_driver_render(int chipset)
{
char *name;
int i, fd;
@@ -307,41 +328,43 @@ static void quiescent_gpu_at_exit_render(int sig)
}
/**
- * drm_open_any:
+ * drm_open_driver:
*
- * Open an i915 drm legacy device node. This function always returns a valid
+ * Open a drm legacy device node. This function always returns a valid
* file descriptor.
*
- * Returns: a i915 drm file descriptor
+ * Returns: a drm file descriptor
*/
-int drm_open_any(void)
+int drm_open_driver(int chipset)
{
static int open_count;
- int fd = __drm_open_any();
+ int fd = __drm_open_driver(chipset);
igt_require(fd >= 0);
if (__sync_fetch_and_add(&open_count, 1))
return fd;
- gem_quiescent_gpu(fd);
- at_exit_drm_fd = __drm_open_any();
- igt_install_exit_handler(quiescent_gpu_at_exit);
+ if(chipset & DRIVER_INTEL){
+ gem_quiescent_gpu(fd);
+ igt_install_exit_handler(quiescent_gpu_at_exit);
+ }
+ at_exit_drm_fd = __drm_open_driver(chipset);
return fd;
}
/**
- * drm_open_any_master:
+ * drm_open_driver_master:
*
- * Open an i915 drm legacy device node and ensure that it is drm master.
+ * Open a drm legacy device node and ensure that it is drm master.
*
* Returns:
- * The i915 drm file descriptor or -1 on error
+ * The drm file descriptor or -1 on error
*/
-int drm_open_any_master(void)
+int drm_open_driver_ma
Re: [Intel-gfx] [PATCH] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
On Tue, Aug 11, 2015 at 06:10:29PM +0300, Ville Syrjälä wrote: > On Tue, Aug 11, 2015 at 05:56:28PM +0300, Ville Syrjälä wrote: > > Hmm. So what would happen on !LLC if we start with a cached bo, then pwrite > > it > > and afterwards make it uncached? > > In fact that would still fail even with my patch, and wouldn't work with > current > upstream code either. To fix that I'd need to drop the I915_CACHE_NONE/WT > checks > from pwrite in my patch and always set cache_dirty=true when it didn't > clflush. Doing that would seem perfectly reasonable to me. Yes. I agree. Marking obj->cache_dirty whenever we dirty the cpu cache and clear it upon clflush seems sane. It will only take effect on transition to the display plane, so not going to impact us except when correctness is required. So maybe we should call it obj->display_dirty? -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
On Tue, Aug 11, 2015 at 05:56:28PM +0300, Ville Syrjälä wrote:
> On Tue, Aug 11, 2015 at 03:28:27PM +0100, Chris Wilson wrote:
> > On Tue, Aug 11, 2015 at 04:59:14PM +0300, ville.syrj...@linux.intel.com
> > wrote:
> > > From: Ville Syrjälä
> > >
> > > Currently we don't clflush on pin_to_display if the bo is already
> > > UC/WT and is not in the CPU write domain. This causes problems with
> > > pwrite since pwrite doesn't change the write domain, and it avoids
> > > clflushing on UC/WT buffers on LLC platforms unless the buffer is
> > > currently being scanned out.
> > >
> > > Fix the problem by marking the cache dirty and adjusting
> > > i915_gem_object_set_cache_level() to clflush when the cache is dirty
> > > even if the cache_level doesn't change.
> > >
> > > My last attempt [1] at fixing this via write domain frobbing was shot
> > > down, but now with the cache_dirty flag we can do things in a nicer way.
> > >
> > > [1]
> > > http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
> > >
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
> > > Testcase: igt/kms_pwrite_crc
> > > Signed-off-by: Ville Syrjälä
> > > ---
> > > drivers/gpu/drm/i915/i915_gem.c | 10 +++---
> > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > > b/drivers/gpu/drm/i915/i915_gem.c
> > > index 73293b4..73eff2e 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > > @@ -1005,12 +1005,15 @@ out:
> > > if (!needs_clflush_after &&
> > > obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
> > > if (i915_gem_clflush_object(obj, obj->pin_display))
> > > - i915_gem_chipset_flush(dev);
> > > + needs_clflush_after = true;
> > > }
> > > }
> > >
> > > if (needs_clflush_after)
> > > i915_gem_chipset_flush(dev);
> > > + else if (obj->cache_level == I915_CACHE_NONE ||
> > > + obj->cache_level == I915_CACHE_WT)
> > > + obj->cache_dirty = true;
> > >
> > > intel_fb_obj_flush(obj, false, ORIGIN_CPU);
> > > return ret;
> >
> > Ok, this seems reasonable.
> >
> > > @@ -3639,10 +3642,10 @@ int i915_gem_object_set_cache_level(struct
> > > drm_i915_gem_object *obj,
> > > {
> > > struct drm_device *dev = obj->base.dev;
> > > struct i915_vma *vma, *next;
> > > - int ret;
> > > + int ret = 0;
> > >
> > > if (obj->cache_level == cache_level)
> > > - return 0;
> > > + goto out;
> > >
> > > if (i915_gem_obj_is_pinned(obj)) {
> > > DRM_DEBUG("can not change the cache level of pinned objects\n");
> > > @@ -3687,6 +3690,7 @@ int i915_gem_object_set_cache_level(struct
> > > drm_i915_gem_object *obj,
> > > vma->node.color = cache_level;
> > > obj->cache_level = cache_level;
> > >
> > > +out:
> > > if (obj->cache_dirty &&
> > > obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
> > > cpu_write_needs_clflush(obj)) {
> >
> > This we can do better (and I know I am guilty of the original sin). It
> > just feels a little too loose that we expect pin-to-display-plane should
> > always call set-cache-level (yes, it has to, but it feels like we are
> > putting pin-to-display-plane specifics into set-cache-level).
> >
> > I think this chunk is much more understandable if we did:
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 5e7fcf77e57a..fc6bcc19cca1 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -3656,13 +3656,6 @@ int i915_gem_object_set_cache_level(struct
> > drm_i915_gem_object *obj,
> > vma->node.color = cache_level;
> > obj->cache_level = cache_level;
> >
> > - if (obj->cache_dirty &&
> > - obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
> > - cpu_write_needs_clflush(obj)) {
> > - if (i915_gem_clflush_object(obj, true))
> > - i915_gem_chipset_flush(obj->base.dev);
> > - }
> > -
> > return 0;
> > }
> >
> > @@ -3795,6 +3788,10 @@ i915_gem_object_pin_to_display_plane(struct
> > drm_i915_gem_object *obj,
> > WARN_ON(obj->pin_display > vma->pin_count);
> >
> > i915_gem_object_flush_cpu_write_domain(obj);
> > + if (obj->cache_dirty) {
> > + if (i915_gem_clflush_object(obj, true))
> > + i915_gem_chipset_flush(obj->base.dev);
> > + }
> >
> > /* It should now be out of any other write domains, and we can
> > update
> > * the domain values for our changes.
> >
> > Maybe even
> >
> > /* Whilst the object was away from the scanout we may have been eliding the
> > coherent
> > * writes into the CPU cache. However, the moment it has to be read by the
> > display
> > * engine, those writes into the CPU cache become inaccessi
Re: [Intel-gfx] [PATCH] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
On Tue, Aug 11, 2015 at 03:28:27PM +0100, Chris Wilson wrote:
> On Tue, Aug 11, 2015 at 04:59:14PM +0300, ville.syrj...@linux.intel.com wrote:
> > From: Ville Syrjälä
> >
> > Currently we don't clflush on pin_to_display if the bo is already
> > UC/WT and is not in the CPU write domain. This causes problems with
> > pwrite since pwrite doesn't change the write domain, and it avoids
> > clflushing on UC/WT buffers on LLC platforms unless the buffer is
> > currently being scanned out.
> >
> > Fix the problem by marking the cache dirty and adjusting
> > i915_gem_object_set_cache_level() to clflush when the cache is dirty
> > even if the cache_level doesn't change.
> >
> > My last attempt [1] at fixing this via write domain frobbing was shot
> > down, but now with the cache_dirty flag we can do things in a nicer way.
> >
> > [1]
> > http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
> > Testcase: igt/kms_pwrite_crc
> > Signed-off-by: Ville Syrjälä
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 10 +++---
> > 1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c
> > b/drivers/gpu/drm/i915/i915_gem.c
> > index 73293b4..73eff2e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -1005,12 +1005,15 @@ out:
> > if (!needs_clflush_after &&
> > obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
> > if (i915_gem_clflush_object(obj, obj->pin_display))
> > - i915_gem_chipset_flush(dev);
> > + needs_clflush_after = true;
> > }
> > }
> >
> > if (needs_clflush_after)
> > i915_gem_chipset_flush(dev);
> > + else if (obj->cache_level == I915_CACHE_NONE ||
> > +obj->cache_level == I915_CACHE_WT)
> > + obj->cache_dirty = true;
> >
> > intel_fb_obj_flush(obj, false, ORIGIN_CPU);
> > return ret;
>
> Ok, this seems reasonable.
>
> > @@ -3639,10 +3642,10 @@ int i915_gem_object_set_cache_level(struct
> > drm_i915_gem_object *obj,
> > {
> > struct drm_device *dev = obj->base.dev;
> > struct i915_vma *vma, *next;
> > - int ret;
> > + int ret = 0;
> >
> > if (obj->cache_level == cache_level)
> > - return 0;
> > + goto out;
> >
> > if (i915_gem_obj_is_pinned(obj)) {
> > DRM_DEBUG("can not change the cache level of pinned objects\n");
> > @@ -3687,6 +3690,7 @@ int i915_gem_object_set_cache_level(struct
> > drm_i915_gem_object *obj,
> > vma->node.color = cache_level;
> > obj->cache_level = cache_level;
> >
> > +out:
> > if (obj->cache_dirty &&
> > obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
> > cpu_write_needs_clflush(obj)) {
>
> This we can do better (and I know I am guilty of the original sin). It
> just feels a little too loose that we expect pin-to-display-plane should
> always call set-cache-level (yes, it has to, but it feels like we are
> putting pin-to-display-plane specifics into set-cache-level).
>
> I think this chunk is much more understandable if we did:
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 5e7fcf77e57a..fc6bcc19cca1 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3656,13 +3656,6 @@ int i915_gem_object_set_cache_level(struct
> drm_i915_gem_object *obj,
> vma->node.color = cache_level;
> obj->cache_level = cache_level;
>
> - if (obj->cache_dirty &&
> - obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
> - cpu_write_needs_clflush(obj)) {
> - if (i915_gem_clflush_object(obj, true))
> - i915_gem_chipset_flush(obj->base.dev);
> - }
> -
> return 0;
> }
>
> @@ -3795,6 +3788,10 @@ i915_gem_object_pin_to_display_plane(struct
> drm_i915_gem_object *obj,
> WARN_ON(obj->pin_display > vma->pin_count);
>
> i915_gem_object_flush_cpu_write_domain(obj);
> + if (obj->cache_dirty) {
> + if (i915_gem_clflush_object(obj, true))
> + i915_gem_chipset_flush(obj->base.dev);
> + }
>
> /* It should now be out of any other write domains, and we can update
> * the domain values for our changes.
>
> Maybe even
>
> /* Whilst the object was away from the scanout we may have been eliding the
> coherent
> * writes into the CPU cache. However, the moment it has to be read by the
> display
> * engine, those writes into the CPU cache become inaccessible and so we have
> to
> * clflush them out to main memory. We track elided flushes with
> obj->cache_dirty
> * and hope they are rare.
> */
>
>
> The other user of set-cache-level (set_caching_ioctl) should not care
> about the clflush side-effect and the
[Intel-gfx] [PATCH v1 2/2] drm/i915/gen9: Disable gather at set shader bit
From Gen9, Push constant instruction parsing behaviour varies according to
whether set shader is enabled or not. If we want legacy behaviour then it
can be achieved by disabling set shader.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89959
Cc: Ben Widawsky
Cc: Joonas Lahtinen
Cc: Mika Kuoppala
Signed-off-by: Arun Siluvery
---
drivers/gpu/drm/i915/i915_reg.h | 5 +
drivers/gpu/drm/i915/intel_ringbuffer.c | 10 ++
2 files changed, 15 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7456bd2..4d32b67 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1720,6 +1720,10 @@ enum skl_disp_power_wells {
#define FW_BLC 0x020d8
#define FW_BLC20x020dc
#define FW_BLC_SELF0x020e0 /* 915+ only */
+#define CS_RCS_BE 0x20D8
+#define CS_RCS_DISABLE_GATHER_AT_SHADER(1<<7)
+#define RS_CHICKEN 0x20DC
+#define RS_CHICKEN_DISABLE_GATHER_AT_SHADER (1<<2)
#define FW_BLC_SELF_EN_MASK (1<<31)
#define FW_BLC_SELF_FIFO_MASK(1<<16) /* 945 only */
#define FW_BLC_SELF_EN (1<<15) /* 945 only */
@@ -5834,6 +5838,7 @@ enum skl_disp_power_wells {
# define GEN7_CSC1_RHWO_OPT_DISABLE_IN_RCC ((1<<10) | (1<<26))
# define GEN9_RHWO_OPTIMIZATION_DISABLE(1<<14)
#define COMMON_SLICE_CHICKEN2 0x7014
+#define GEN9_DISABLE_GATHER_SET_SHADER_SLICE (1<<12)
# define GEN8_CSC2_SBE_VUE_CACHE_CONSERVATIVE (1<<0)
#define HIZ_CHICKEN0x7018
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index cf61262..7d284ed 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -983,6 +983,16 @@ static int gen9_init_workarounds(struct intel_engine_cs
*ring)
tmp |= HDC_FORCE_CSR_NON_COHERENT_OVR_DISABLE;
WA_SET_BIT_MASKED(HDC_CHICKEN0, tmp);
+ /* Chicken bits to disable set shader is in multiple places,
+* set bits in all required registers to disable it correctly
+*/
+ WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,
GEN9_DISABLE_GATHER_SET_SHADER_SLICE);
+ if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) <= SKL_REVID_D0) ||
+ (IS_BROXTON(dev) && INTEL_REVID(dev) == BXT_REVID_A0))
+ WA_SET_BIT_MASKED(RS_CHICKEN,
RS_CHICKEN_DISABLE_GATHER_AT_SHADER);
+ else
+ WA_SET_BIT_MASKED(CS_RCS_BE, CS_RCS_DISABLE_GATHER_AT_SHADER);
+
return 0;
}
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v1 1/2] drm/i915: Contain the WA_REG macro
From: Mika Kuoppala
Prevent leaking the if scoping by containing the WA_REG
macro inside its own scope.
Reported-by: Arun Siluvery
Signed-off-by: Mika Kuoppala
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1c14233..cf61262 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -780,11 +780,11 @@ static int wa_add(struct drm_i915_private *dev_priv,
return 0;
}
-#define WA_REG(addr, mask, val) { \
+#define WA_REG(addr, mask, val) do { \
const int r = wa_add(dev_priv, (addr), (mask), (val)); \
if (r) \
return r; \
- }
+ } while(0)
#define WA_SET_BIT_MASKED(addr, mask) \
WA_REG(addr, (mask), _MASKED_BIT_ENABLE(mask))
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v1 0/2] Enable legacy behaviour for Push constants
Patch1 fixes a simple compile error in Patch2 Patch2 fixes gpu hang observed with a subtest of gem_concurrent_blit. Arun Siluvery (1): drm/i915/gen9: Disable gather at set shader bit Mika Kuoppala (1): drm/i915: Contain the WA_REG macro drivers/gpu/drm/i915/i915_reg.h | 5 + drivers/gpu/drm/i915/intel_ringbuffer.c | 14 -- 2 files changed, 17 insertions(+), 2 deletions(-) -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915: Change SRM, LRM instructions to use correct length
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 7001 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -1 302/302 301/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT -1 283/283 282/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1) *BYT igt@gem_tiled_partial_pwrite_pread@reads PASS(1) FAIL(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
On Tue, Aug 11, 2015 at 04:59:14PM +0300, ville.syrj...@linux.intel.com wrote:
> From: Ville Syrjälä
>
> Currently we don't clflush on pin_to_display if the bo is already
> UC/WT and is not in the CPU write domain. This causes problems with
> pwrite since pwrite doesn't change the write domain, and it avoids
> clflushing on UC/WT buffers on LLC platforms unless the buffer is
> currently being scanned out.
>
> Fix the problem by marking the cache dirty and adjusting
> i915_gem_object_set_cache_level() to clflush when the cache is dirty
> even if the cache_level doesn't change.
>
> My last attempt [1] at fixing this via write domain frobbing was shot
> down, but now with the cache_dirty flag we can do things in a nicer way.
>
> [1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
> Testcase: igt/kms_pwrite_crc
> Signed-off-by: Ville Syrjälä
> ---
> drivers/gpu/drm/i915/i915_gem.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 73293b4..73eff2e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1005,12 +1005,15 @@ out:
> if (!needs_clflush_after &&
> obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
> if (i915_gem_clflush_object(obj, obj->pin_display))
> - i915_gem_chipset_flush(dev);
> + needs_clflush_after = true;
> }
> }
>
> if (needs_clflush_after)
> i915_gem_chipset_flush(dev);
> + else if (obj->cache_level == I915_CACHE_NONE ||
> + obj->cache_level == I915_CACHE_WT)
> + obj->cache_dirty = true;
>
> intel_fb_obj_flush(obj, false, ORIGIN_CPU);
> return ret;
Ok, this seems reasonable.
> @@ -3639,10 +3642,10 @@ int i915_gem_object_set_cache_level(struct
> drm_i915_gem_object *obj,
> {
> struct drm_device *dev = obj->base.dev;
> struct i915_vma *vma, *next;
> - int ret;
> + int ret = 0;
>
> if (obj->cache_level == cache_level)
> - return 0;
> + goto out;
>
> if (i915_gem_obj_is_pinned(obj)) {
> DRM_DEBUG("can not change the cache level of pinned objects\n");
> @@ -3687,6 +3690,7 @@ int i915_gem_object_set_cache_level(struct
> drm_i915_gem_object *obj,
> vma->node.color = cache_level;
> obj->cache_level = cache_level;
>
> +out:
> if (obj->cache_dirty &&
> obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
> cpu_write_needs_clflush(obj)) {
This we can do better (and I know I am guilty of the original sin). It
just feels a little too loose that we expect pin-to-display-plane should
always call set-cache-level (yes, it has to, but it feels like we are
putting pin-to-display-plane specifics into set-cache-level).
I think this chunk is much more understandable if we did:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 5e7fcf77e57a..fc6bcc19cca1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3656,13 +3656,6 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
vma->node.color = cache_level;
obj->cache_level = cache_level;
- if (obj->cache_dirty &&
- obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
- cpu_write_needs_clflush(obj)) {
- if (i915_gem_clflush_object(obj, true))
- i915_gem_chipset_flush(obj->base.dev);
- }
-
return 0;
}
@@ -3795,6 +3788,10 @@ i915_gem_object_pin_to_display_plane(struct
drm_i915_gem_object *obj,
WARN_ON(obj->pin_display > vma->pin_count);
i915_gem_object_flush_cpu_write_domain(obj);
+ if (obj->cache_dirty) {
+ if (i915_gem_clflush_object(obj, true))
+ i915_gem_chipset_flush(obj->base.dev);
+ }
/* It should now be out of any other write domains, and we can update
* the domain values for our changes.
Maybe even
/* Whilst the object was away from the scanout we may have been eliding the
coherent
* writes into the CPU cache. However, the moment it has to be read by the
display
* engine, those writes into the CPU cache become inaccessible and so we have
to
* clflush them out to main memory. We track elided flushes with
obj->cache_dirty
* and hope they are rare.
*/
The other user of set-cache-level (set_caching_ioctl) should not care
about the clflush side-effect and the clflush elision should work just
fine instead.
Either way,
Reviewed-by: Chris Wilson
but I'd prefer a v2 with the comments :)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@l
[Intel-gfx] [PATCH i-g-t] tests/kms_pwrite_crc: Use drmModeSetPlane() instead of igt_plane_set_fb()
From: Ville Syrjälä
igt_plane_set_fb()+igt_display_commit() have too much overhead, and that
causes the cache to get flushed before we flip, making the test
useless, at least on machines with small LLC. Switch to
drmModeSetPlane() to reduce the chance that the cache gets flushed
before we grab the crc.
Still nowhere near 100% reliable on my IVB laptop with 3 MiB LLC,
but at least it can now hit the problem occasioanally. My desktop
IVB with 8 MiB LLC seems to hit it rather reliably.
Signed-off-by: Ville Syrjälä
---
tests/kms_pwrite_crc.c | 25 ++---
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/tests/kms_pwrite_crc.c b/tests/kms_pwrite_crc.c
index 05b9e38..990866f 100644
--- a/tests/kms_pwrite_crc.c
+++ b/tests/kms_pwrite_crc.c
@@ -52,7 +52,6 @@ typedef struct {
static void test(data_t *data)
{
- igt_display_t *display = &data->display;
igt_output_t *output = data->output;
struct igt_fb *fb = &data->fb[1];
drmModeModeInfo *mode;
@@ -72,12 +71,20 @@ static void test(data_t *data)
cairo_destroy(cr);
/* flip to it to make it UC/WC and fully flushed */
- igt_plane_set_fb(data->primary, fb);
- igt_display_commit(display);
+ drmModeSetPlane(data->drm_fd,
+ data->primary->drm_plane->plane_id,
+ output->config.crtc->crtc_id,
+ fb->fb_id, 0,
+ 0, 0, fb->width, fb->height,
+ 0, 0, fb->width << 16, fb->height << 16);
/* flip back the original white buffer */
- igt_plane_set_fb(data->primary, &data->fb[0]);
- igt_display_commit(display);
+ drmModeSetPlane(data->drm_fd,
+ data->primary->drm_plane->plane_id,
+ output->config.crtc->crtc_id,
+ data->fb[0].fb_id, 0,
+ 0, 0, fb->width, fb->height,
+ 0, 0, fb->width << 16, fb->height << 16);
/* make sure caching mode has become UC/WT */
caching = gem_get_caching(data->drm_fd, fb->gem_handle);
@@ -91,8 +98,12 @@ static void test(data_t *data)
free(buf);
/* and flip to it */
- igt_plane_set_fb(data->primary, fb);
- igt_display_commit(display);
+ drmModeSetPlane(data->drm_fd,
+ data->primary->drm_plane->plane_id,
+ output->config.crtc->crtc_id,
+ fb->fb_id, 0,
+ 0, 0, fb->width, fb->height,
+ 0, 0, fb->width << 16, fb->height << 16);
/* check that the crc is as expected, which requires that caches got
flushed */
igt_pipe_crc_collect_crc(data->pipe_crc, &crc);
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: clflush on pin_to_display after pwrite to UC bo in LLC
From: Ville Syrjälä
Currently we don't clflush on pin_to_display if the bo is already
UC/WT and is not in the CPU write domain. This causes problems with
pwrite since pwrite doesn't change the write domain, and it avoids
clflushing on UC/WT buffers on LLC platforms unless the buffer is
currently being scanned out.
Fix the problem by marking the cache dirty and adjusting
i915_gem_object_set_cache_level() to clflush when the cache is dirty
even if the cache_level doesn't change.
My last attempt [1] at fixing this via write domain frobbing was shot
down, but now with the cache_dirty flag we can do things in a nicer way.
[1] http://lists.freedesktop.org/archives/intel-gfx/2014-November/055390.html
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86422
Testcase: igt/kms_pwrite_crc
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_gem.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73293b4..73eff2e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1005,12 +1005,15 @@ out:
if (!needs_clflush_after &&
obj->base.write_domain != I915_GEM_DOMAIN_CPU) {
if (i915_gem_clflush_object(obj, obj->pin_display))
- i915_gem_chipset_flush(dev);
+ needs_clflush_after = true;
}
}
if (needs_clflush_after)
i915_gem_chipset_flush(dev);
+ else if (obj->cache_level == I915_CACHE_NONE ||
+obj->cache_level == I915_CACHE_WT)
+ obj->cache_dirty = true;
intel_fb_obj_flush(obj, false, ORIGIN_CPU);
return ret;
@@ -3639,10 +3642,10 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
{
struct drm_device *dev = obj->base.dev;
struct i915_vma *vma, *next;
- int ret;
+ int ret = 0;
if (obj->cache_level == cache_level)
- return 0;
+ goto out;
if (i915_gem_obj_is_pinned(obj)) {
DRM_DEBUG("can not change the cache level of pinned objects\n");
@@ -3687,6 +3690,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
vma->node.color = cache_level;
obj->cache_level = cache_level;
+out:
if (obj->cache_dirty &&
obj->base.write_domain != I915_GEM_DOMAIN_CPU &&
cpu_write_needs_clflush(obj)) {
--
2.4.6
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [BUGFIX] drm/i915: Fix for VBT expected size
Depending on the VBT BDB version the maximum size
can be up to 38 bytes.
This fix increases the maximum of the VBT expected size
from 33 bytes to 38 bytes and by doing so cures the kernel
hang on BSW box.
Signed-off-by: Mika Kahola
---
drivers/gpu/drm/i915/intel_bios.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_bios.h
b/drivers/gpu/drm/i915/intel_bios.h
index f7ad6a5..788463d 100644
--- a/drivers/gpu/drm/i915/intel_bios.h
+++ b/drivers/gpu/drm/i915/intel_bios.h
@@ -246,7 +246,7 @@ struct common_child_dev_config {
union child_device_config {
/* This one is safe to be used anywhere, but the code should still check
* the BDB version. */
- u8 raw[33];
+ u8 raw[38];
/* This one should only be kept for legacy code. */
struct old_child_dev_config old;
/* This one should also be safe to use anywhere, even without version
--
1.9.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Treat foreign dma-buf imports as uncached
On Tue, Aug 11, 2015 at 02:58:59PM +0200, Daniel Vetter wrote: > On Tue, Aug 11, 2015 at 11:24:58AM +0100, Chris Wilson wrote: > > On Tue, Aug 11, 2015 at 12:12:08PM +0200, Daniel Vetter wrote: > > > On Mon, Aug 10, 2015 at 09:16:46PM +0100, Chris Wilson wrote: > > > > If the set of pages is being imported from another device, we cannot > > > > assume that it is fully coherent with the CPU cache, so mark it as such. > > > > However, if the source is the shared memory vgem allocator, we could > > > > treat the buffer as being cached (so long as all parties agree in the > > > > case the same buffer is shared between multiple devices) but as of > > > > today, vgem cannot export dma-bufs. > > > > > > > > Signed-off-by: Chris Wilson > > > > Cc: Daniel Vetter > > > > > > Yay for x86 dma api to refuse acknowleding that incoherent devices exist > > > (since this should be done in the dma-ap/sg stuff imo for dma-bufs). > > > > > > But since x86 assumes that everything is coherent shouldn't we do the > > > inverse and use snooping/cacheable always? > > > > Actually, the convention for PRIME is that transfer bos are uncached > > because we have to be sure that any write makes it into the foriegn > > pages, that applies to both GPU and CPU writes, precisely because the > > target is *incoherent*. > > > > > That should be correct for everything modern at least, and for old agp > > > crap (do we care even, is sharing possible there?) snooping should only > > > result in a bit more overhead. > > > > > > Or where exactly does this blow up? > > > > Consider a kms_crc-esque test using a radeon/nouveau bo imported into > > i915 and accessed using i915 ioctls (with the CRC testing on the radeon > > scanout). > > Scanout on radeon must be in vram afaik, and that's a place i915 can't get > at. I think that even holds true for the integrated radones (they just use > stolen for vram then). > > The other way round also doesn't work I think since i915 can't change the > caching policy for radoen/nouveau access if radeon/nouveau want to write > directly to main memory. Otoh I think pcie transactions just snoop the > cache and never put anything in there. > > I still think that everything we import and don't have a clue about should > probably be treated as snooped conceptually. But practically who knows > what's going on ... I'd say setting as uncached is safer. But at the moment, the policy is just whatever the platform default is and that leads to interesting platform specific behaviour. At the very least we do want snooping if we ever do get a shmem dma-buf source. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Treat foreign dma-buf imports as uncached
On Tue, Aug 11, 2015 at 11:24:58AM +0100, Chris Wilson wrote: > On Tue, Aug 11, 2015 at 12:12:08PM +0200, Daniel Vetter wrote: > > On Mon, Aug 10, 2015 at 09:16:46PM +0100, Chris Wilson wrote: > > > If the set of pages is being imported from another device, we cannot > > > assume that it is fully coherent with the CPU cache, so mark it as such. > > > However, if the source is the shared memory vgem allocator, we could > > > treat the buffer as being cached (so long as all parties agree in the > > > case the same buffer is shared between multiple devices) but as of > > > today, vgem cannot export dma-bufs. > > > > > > Signed-off-by: Chris Wilson > > > Cc: Daniel Vetter > > > > Yay for x86 dma api to refuse acknowleding that incoherent devices exist > > (since this should be done in the dma-ap/sg stuff imo for dma-bufs). > > > > But since x86 assumes that everything is coherent shouldn't we do the > > inverse and use snooping/cacheable always? > > Actually, the convention for PRIME is that transfer bos are uncached > because we have to be sure that any write makes it into the foriegn > pages, that applies to both GPU and CPU writes, precisely because the > target is *incoherent*. > > > That should be correct for everything modern at least, and for old agp > > crap (do we care even, is sharing possible there?) snooping should only > > result in a bit more overhead. > > > > Or where exactly does this blow up? > > Consider a kms_crc-esque test using a radeon/nouveau bo imported into > i915 and accessed using i915 ioctls (with the CRC testing on the radeon > scanout). Scanout on radeon must be in vram afaik, and that's a place i915 can't get at. I think that even holds true for the integrated radones (they just use stolen for vram then). The other way round also doesn't work I think since i915 can't change the caching policy for radoen/nouveau access if radeon/nouveau want to write directly to main memory. Otoh I think pcie transactions just snoop the cache and never put anything in there. I still think that everything we import and don't have a clue about should probably be treated as snooped conceptually. But practically who knows what's going on ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Clean up lrc context init
On Fri, Aug 07, 2015 at 11:05:24AM +0100, Nick Hoath wrote:
> Clean up lrc context init by:
>- Move context initialisation in to i915_gem_init_hw
>- Move one off initialisation for render ring to
> i915_gem_validate_context
>- Move default context initialisation to logical_ring_init
>
> Rename intel_lr_context_deferred_create to
> intel_lr_context_deferred_alloc, to reflect reduced functionality.
>
> Issue: VIZ-4798
> Signed-off-by: Nick Hoath
Commit message is a bit thin since it only describes what the patch does
and not why. Goal here is to put the init/init_hw split we have in all
other places into place for lrc context init, i.e. allocate resources
in one function (for driver load) and setup all the hw state in another
function (used both for driver load, resume and after gpu reset).
I think this patch achieves this, with the few minor comments below
addressed. So looks like a step in the right direction.
We still have a bit a mess in init ordering and a lot of if (execlist) all
over the place, but imo that should be done separately.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h| 1 -
> drivers/gpu/drm/i915/i915_gem.c| 23 ++---
> drivers/gpu/drm/i915/i915_gem_context.c| 21 -
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 36 +++-
> drivers/gpu/drm/i915/intel_lrc.c | 133
> +++--
> drivers/gpu/drm/i915/intel_lrc.h | 4 +-
> drivers/gpu/drm/i915/intel_ringbuffer.c| 2 +
> drivers/gpu/drm/i915/intel_ringbuffer.h| 1 +
> 8 files changed, 105 insertions(+), 116 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 92ea9ad..aa6c6dc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3065,7 +3065,6 @@ int __must_check i915_gem_context_init(struct
> drm_device *dev);
> void i915_gem_context_fini(struct drm_device *dev);
> void i915_gem_context_reset(struct drm_device *dev);
> int i915_gem_context_open(struct drm_device *dev, struct drm_file *file);
> -int i915_gem_context_enable(struct drm_i915_gem_request *req);
> void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
> int i915_switch_context(struct drm_i915_gem_request *req);
> struct intel_context *
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d9f2701..7ebc6e3 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -4996,14 +4996,8 @@ int i915_gem_init_rings(struct drm_device *dev)
> goto cleanup_vebox_ring;
> }
>
> - ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
> - if (ret)
> - goto cleanup_bsd2_ring;
> -
> return 0;
>
> -cleanup_bsd2_ring:
> - intel_cleanup_ring_buffer(&dev_priv->ring[VCS2]);
> cleanup_vebox_ring:
> intel_cleanup_ring_buffer(&dev_priv->ring[VECS]);
> cleanup_blt_ring:
> @@ -5022,6 +5016,7 @@ i915_gem_init_hw(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_engine_cs *ring;
> int ret, i, j;
> + struct drm_i915_gem_request *req;
>
> if (INTEL_INFO(dev)->gen < 6 && !intel_enable_gtt())
> return -EIO;
> @@ -5073,9 +5068,12 @@ i915_gem_init_hw(struct drm_device *dev)
> goto out;
> }
>
> + ret = i915_gem_set_seqno(dev, ((u32)~0 - 0x1000));
> + if (ret)
> + goto out;
> +
> /* Now it is safe to go back round and do everything else: */
> for_each_ring(ring, dev_priv, i) {
> - struct drm_i915_gem_request *req;
>
> WARN_ON(!ring->default_context);
>
> @@ -5098,9 +5096,14 @@ i915_gem_init_hw(struct drm_device *dev)
> goto out;
> }
>
> - ret = i915_gem_context_enable(req);
> - if (ret && ret != -EIO) {
> - DRM_ERROR("Context enable ring #%d failed %d\n", i,
> ret);
> + ret = 0;
> + if (ring->switch_context != NULL) {
> + ret = ring->switch_context(req);
> + } else if (ring->init_context != NULL) {
> + ret = ring->init_context(req);
> + }
Adding switch_context as a replacement for the if(execlist) in
gem_context_enable doesn't seem to help in clarity but does add another
indirection. I'd just drop it.
> + if (ret) {
> + DRM_ERROR("ring init context: %d\n", ret);
> i915_gem_request_cancel(req);
> i915_gem_cleanup_ringbuffer(dev);
> goto out;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
> b/drivers/gpu/drm/i915/i915_gem_context.c
> index b77a8f7..18bd5f5 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -407,27 +407,6 @@ void i915_gem_conte
[Intel-gfx] [PATCH] lib/rendercopy_gen9: WaBindlessSurfaceStateModifyEnable
Don't set the size of bindless surface state on rendercopy.
And as of doing so, take into account the workaround for setting
the command size.
This was tried during hunting for
https://bugs.freedesktop.org/show_bug.cgi?id=89959. But no
impact was found.
Cc: Arun Siluvery
Signed-off-by: Mika Kuoppala
---
lib/rendercopy_gen9.c | 10 +++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/lib/rendercopy_gen9.c b/lib/rendercopy_gen9.c
index 0766192..4a4a604 100644
--- a/lib/rendercopy_gen9.c
+++ b/lib/rendercopy_gen9.c
@@ -511,7 +511,11 @@ gen7_emit_push_constants(struct intel_batchbuffer *batch) {
static void
gen9_emit_state_base_address(struct intel_batchbuffer *batch) {
- OUT_BATCH(GEN6_STATE_BASE_ADDRESS | (19 - 2));
+
+ /* WaBindlessSurfaceStateModifyEnable:skl,bxt */
+ /* The length has to be one less if we dont modify
+ bindless state */
+ OUT_BATCH(GEN6_STATE_BASE_ADDRESS | (19 - 1 - 2));
/* general */
OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
@@ -544,9 +548,9 @@ gen9_emit_state_base_address(struct intel_batchbuffer
*batch) {
OUT_BATCH(1 << 12 | 1);
/* Bindless surface state base address */
- OUT_BATCH(0 | BASE_ADDRESS_MODIFY);
OUT_BATCH(0);
- OUT_BATCH(0xf000);
+ OUT_BATCH(0);
+ OUT_BATCH(0);
}
static void
--
2.1.4
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
On Tue, Aug 11, 2015 at 01:25:08PM +0200, Maarten Lankhorst wrote: > Op 10-08-15 om 13:34 schreef Daniel Vetter: > > Instead of our own duplicated one. This fixes a bug in the driver > > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > > try to unregister the nonexistent fbdev drm_framebuffer. > > > > Cc: Archit Taneja > > Cc: Maarten Lankhorst > > Reported-by: Maarten Lankhorst > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/i915/Kconfig | 15 --- > > drivers/gpu/drm/i915/Makefile| 2 +- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 7 files changed, 8 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > > index eb87e2538861..051eab33e4c7 100644 > > --- a/drivers/gpu/drm/i915/Kconfig > > +++ b/drivers/gpu/drm/i915/Kconfig > > @@ -36,21 +36,6 @@ config DRM_I915 > > i810 driver instead, and the Atom z5xx series has an entirely > > different implementation. > > > > -config DRM_I915_FBDEV > > - bool "Enable legacy fbdev support for the modesetting intel driver" > > - depends on DRM_I915 > > - select DRM_KMS_FB_HELPER > > - select FB_CFB_FILLRECT > > - select FB_CFB_COPYAREA > > - select FB_CFB_IMAGEBLIT > > > Did some testing and didn't find a way to break this. Has the bug been fixed > where if some > option selects X that selects Y it also had to select Y? Nope, but DRM_FBDEV_EMULATION should carry all the same selects as we did. > > Oh well, have my r-b. :-) > > Reviewed-by: Maarten Lankhorst Thanks, applied to drm-misc. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6 v2] drm/i915: Enable HDMI on DDI-E
On Mon, Aug 10, 2015 at 03:02:44PM +0800, Xiong Zhang wrote:
> DDI-E doesn't have the correspondent GMBUS pin.
>
> We rely on VBT to tell us which one it being used instead.
>
> The DVI/HDMI on shared port couldn't exist.
>
> This patch isn't tested without hardware wchich has HDMI
> on DDI-E.
>
> v2: fix trailing whitespace
>
> Signed-off-by: Xiong Zhang
> Reviewed-by: Rodrigo Vivi
When resending individual patches from a series please use --in-reply-to
to link the them up with the original patch submission.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +
> drivers/gpu/drm/i915/intel_bios.c | 25 +
> drivers/gpu/drm/i915/intel_hdmi.c | 18 ++
> 3 files changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6d93334..5d8e7fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1414,6 +1414,10 @@ enum modeset_restore {
> #define DP_AUX_C 0x20
> #define DP_AUX_D 0x30
>
> +#define DDC_PIN_B 0x05
> +#define DDC_PIN_C 0x04
> +#define DDC_PIN_D 0x06
> +
> struct ddi_vbt_port_info {
> /*
>* This is an index in the HDMI/DVI DDI buffer translation table.
> @@ -1428,6 +1432,7 @@ struct ddi_vbt_port_info {
> uint8_t supports_dp:1;
>
> uint8_t alternate_aux_channel;
> + uint8_t alternate_ddc_pin;
> };
>
> enum psr_lines_to_wait {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 16cdf17..8140531 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -894,7 +894,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> uint8_t hdmi_level_shift;
> int i, j;
> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> - uint8_t aux_channel;
> + uint8_t aux_channel, ddc_pin;
> /* Each DDI port can have more than one value on the "DVO Port" field,
>* so look for all the possible values for each port and abort if more
>* than one is found. */
> @@ -928,6 +928,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> return;
>
> aux_channel = child->raw[25];
> + ddc_pin = child->common.ddc_pin;
>
> is_dvi = child->common.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
> is_dp = child->common.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> @@ -959,11 +960,27 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>
> if (is_dvi) {
> - if (child->common.ddc_pin == 0x05 && port != PORT_B)
> + if (port == PORT_E) {
> + info->alternate_ddc_pin = ddc_pin;
> + /* if DDIE share ddc pin with other port, then
> + * dvi/hdmi couldn't exist on the shared port.
> + * Otherwise they share the same ddc bin and system
> + * couldn't communicate with them seperately. */
> + if (ddc_pin == DDC_PIN_B) {
> +
> dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> + } else if (ddc_pin == DDC_PIN_C) {
> +
> dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> + } else if (ddc_pin == DDC_PIN_D) {
> +
> dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> + }
> + } else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> - if (child->common.ddc_pin == 0x04 && port != PORT_C)
> + else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> - if (child->common.ddc_pin == 0x06 && port != PORT_D)
> + else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..e1f6e81 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1991,6 +1991,24 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> intel_encoder->hpd_pin = HPD_PORT_D;
> break;
> + case PORT_E:
> + /* On SKL PORT E doesn't have seperate GMBUS pin
>
Re: [Intel-gfx] [PATCH 1/2] drm/i915: calculate primary visibility changes instead of calling from set_config
Op 11-08-15 om 13:12 schreef Jani Nikula: > On Tue, 11 Aug 2015, Maarten Lankhorst > wrote: >> This should be much cleaner, with the same effects. >> >> (cherry picked from commit fb9d6cf8c29bfcb0b3c602f7ded87f128d730382) >> Signed-off-by: Maarten Lankhorst >> Reviewed-by: Matt Roper >> Signed-off-by: Jani Nikula >> Cc: sta...@vger.kernel.org #v4.2 > I don't understand. v4.2 has not been released, and we can still queue > fixes for the release. > Ok in that case just queue both patches for v4.2. :) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
Op 10-08-15 om 13:34 schreef Daniel Vetter: > Instead of our own duplicated one. This fixes a bug in the driver > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > try to unregister the nonexistent fbdev drm_framebuffer. > > Cc: Archit Taneja > Cc: Maarten Lankhorst > Reported-by: Maarten Lankhorst > Signed-off-by: Daniel Vetter > --- > drivers/gpu/drm/i915/Kconfig | 15 --- > drivers/gpu/drm/i915/Makefile| 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 7 files changed, 8 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index eb87e2538861..051eab33e4c7 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -36,21 +36,6 @@ config DRM_I915 > i810 driver instead, and the Atom z5xx series has an entirely > different implementation. > > -config DRM_I915_FBDEV > - bool "Enable legacy fbdev support for the modesetting intel driver" > - depends on DRM_I915 > - select DRM_KMS_FB_HELPER > - select FB_CFB_FILLRECT > - select FB_CFB_COPYAREA > - select FB_CFB_IMAGEBLIT > Did some testing and didn't find a way to break this. Has the bug been fixed where if some option selects X that selects Y it also had to select Y? Oh well, have my r-b. :-) Reviewed-by: Maarten Lankhorst ~Maarten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: calculate primary visibility changes instead of calling from set_config
On Tue, 11 Aug 2015, Maarten Lankhorst
wrote:
> This should be much cleaner, with the same effects.
>
> (cherry picked from commit fb9d6cf8c29bfcb0b3c602f7ded87f128d730382)
> Signed-off-by: Maarten Lankhorst
> Reviewed-by: Matt Roper
> Signed-off-by: Jani Nikula
> Cc: sta...@vger.kernel.org #v4.2
I don't understand. v4.2 has not been released, and we can still queue
fixes for the release.
BR,
Jani.
> References: https://bugs.freedesktop.org/show_bug.cgi?id=90398
> ---
> drivers/gpu/drm/i915/intel_display.c | 46
> ++--
> 1 file changed, 7 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 30e0f54ba19d..c2579ded0c36 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12891,20 +12891,11 @@ intel_modeset_stage_output_state(struct drm_device
> *dev,
> return 0;
> }
>
> -static bool primary_plane_visible(struct drm_crtc *crtc)
> -{
> - struct intel_plane_state *plane_state =
> - to_intel_plane_state(crtc->primary->state);
> -
> - return plane_state->visible;
> -}
> -
> static int intel_crtc_set_config(struct drm_mode_set *set)
> {
> struct drm_device *dev;
> struct drm_atomic_state *state = NULL;
> struct intel_crtc_state *pipe_config;
> - bool primary_plane_was_visible;
> int ret;
>
> BUG_ON(!set);
> @@ -12943,38 +12934,8 @@ static int intel_crtc_set_config(struct drm_mode_set
> *set)
>
> intel_update_pipe_size(to_intel_crtc(set->crtc));
>
> - primary_plane_was_visible = primary_plane_visible(set->crtc);
> -
> ret = intel_set_mode_with_config(set->crtc, pipe_config, true);
>
> - if (ret == 0 &&
> - pipe_config->base.enable &&
> - pipe_config->base.planes_changed &&
> - !needs_modeset(&pipe_config->base)) {
> - struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
> -
> - /*
> - * We need to make sure the primary plane is re-enabled if it
> - * has previously been turned off.
> - */
> - if (ret == 0 && !primary_plane_was_visible &&
> - primary_plane_visible(set->crtc)) {
> - WARN_ON(!intel_crtc->active);
> - intel_post_enable_primary(set->crtc);
> - }
> -
> - /*
> - * In the fastboot case this may be our only check of the
> - * state after boot. It would be better to only do it on
> - * the first update, but we don't have a nice way of doing that
> - * (and really, set_config isn't used much for high freq page
> - * flipping, so increasing its cost here shouldn't be a big
> - * deal).
> - */
> - if (i915.fastboot && ret == 0)
> - intel_modeset_check_state(set->crtc->dev);
> - }
> -
> if (ret) {
> DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
> set->crtc->base.id, ret);
> @@ -13305,6 +13266,9 @@ intel_check_primary_plane(struct drm_plane *plane,
>*/
> if (IS_BROADWELL(dev))
> intel_crtc->atomic.wait_vblank = true;
> +
> + if (crtc_state && !needs_modeset(&crtc_state->base))
> + intel_crtc->atomic.post_enable_primary = true;
> }
>
> /*
> @@ -13317,6 +13281,10 @@ intel_check_primary_plane(struct drm_plane *plane,
> if (!state->visible || !fb)
> intel_crtc->atomic.disable_ips = true;
>
> + if (!state->visible && old_state->visible &&
> + crtc_state && !needs_modeset(&crtc_state->base))
> + intel_crtc->atomic.pre_disable_primary = true;
> +
> intel_crtc->atomic.fb_bits |=
> INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
>
> --
> 2.1.0
>
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
HI Daniel, Looks like we are not getting an IVB machine here. I think instead of blocking the patch set for the whole series, we can just block it for the platforms we don’t get success for. We are shipping many VLV/CHV systems with these patches applied, and they are passing all Android test suits and test benches, so we don’t doubt the solution as such. We have done testing for following platforms (gen 9 is tested by Imre also): 1. CHV / BSW 2. SKL 3. BXT 4. BDW Why don’t we go ahead with this solution for gen 8 onwards, and keep the rest of the code un touched, this will serve both the purpose. Regards Shashank -Original Message- From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of Daniel Vetter Sent: Tuesday, August 11, 2015 3:12 PM To: Jindal, Sonika Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote: > > > On 8/10/2015 1:38 PM, Daniel Vetter wrote: > >On Mon, Aug 10, 2015 at 04:50:37AM +, Jindal, Sonika wrote: > >>Hi Daniel, > >> > >>That patch was already merged: > >>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.htm > >>l > >> > >>For SKL, the above patch helped in getting the correct ISR bits set. > >>One option is to enable the HDMI optimization from VLV onwards. > >>I don't have an ivb machine to try out the issue. > > > >ivb is simple the machine I have here, but when we tried this the > >last time around we had reports for all platforms (your patch still > >doesn't cite the relevant sha1 btw). I think there are 3 possible > >explanations: > > > Yes, I don't know which were those patches and how to find them.. git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code. > >a) we do something wrong with hpd handling on these platforms. That > >seems to be the explanation you favour (with the gen >= 7 checks and > >all that), but I think it's very unlikely: On each platform where we > >had reports of hpd being broken there was also machines where hpd works > >perfectly fine. > > > Not sure, I will find one ivb system and try on that. > > >b) There's broken HDMI (or DVI) sinks out there. If that's the case > >we can never merge your patch. > I doubt this because we have tested these patches with many sinks in > the past with VLV/CHV. > > > >c) There's something in vbt or somewhere else that tells the windows > >driver whether using hpd or not is ok (and the hpd problem is > >actually an issue with certain OEM machines ...). > > > No, nothing like that. > > >I hoped that with your hpd handling fix we'd have some indication > >that our hpd troubles are of type a). But since I tested with your > >patch that didn't work out. > > > >And until we have some evidence that our hpd troubles aren't type b) > >I really don't want to merge any patch which relies upon hpd bits for hdmi. > >-Daniel > > > I will try on ivb. I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Use CONFIG_DRM_FBDEV_EMULATION
Op 10-08-15 om 13:34 schreef Daniel Vetter:
> Instead of our own duplicated one. This fixes a bug in the driver
> unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we
> try to unregister the nonexistent fbdev drm_framebuffer.
>
> Cc: Archit Taneja
> Cc: Maarten Lankhorst
> Reported-by: Maarten Lankhorst
> Signed-off-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/Kconfig | 15 ---
> drivers/gpu/drm/i915/Makefile| 2 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 2 +-
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/intel_display.c | 4 ++--
> drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++--
> drivers/gpu/drm/i915/intel_drv.h | 2 +-
> 7 files changed, 8 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
> index eb87e2538861..051eab33e4c7 100644
> --- a/drivers/gpu/drm/i915/Kconfig
> +++ b/drivers/gpu/drm/i915/Kconfig
> @@ -36,21 +36,6 @@ config DRM_I915
> i810 driver instead, and the Atom z5xx series has an entirely
> different implementation.
>
> -config DRM_I915_FBDEV
> - bool "Enable legacy fbdev support for the modesetting intel driver"
> - depends on DRM_I915
> - select DRM_KMS_FB_HELPER
> - select FB_CFB_FILLRECT
> - select FB_CFB_COPYAREA
> - select FB_CFB_IMAGEBLIT
> - default y
> - help
> - Choose this option if you have a need for the legacy fbdev
> - support. Note that this support also provide the linux console
> - support on top of the intel modesetting driver.
> -
> - If in doubt, say "Y".
> -
> config DRM_I915_PRELIMINARY_HW_SUPPORT
> bool "Enable preliminary support for prerelease Intel hardware by
> default"
> depends on DRM_I915
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 41fb8a9c5bef..998b4643109f 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -62,7 +62,7 @@ i915-y += intel_audio.o \
> intel_sideband.o \
> intel_sprite.o
> i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o
> -i915-$(CONFIG_DRM_I915_FBDEV)+= intel_fbdev.o
> +i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o
>
> # modesetting output/encoder code
> i915-y += dvo_ch7017.o \
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 95e7b82f05d1..86734be84a65 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1868,7 +1868,7 @@ static int i915_gem_framebuffer_info(struct seq_file
> *m, void *data)
> struct intel_framebuffer *fb;
> struct drm_framebuffer *drm_fb;
>
> -#ifdef CONFIG_DRM_I915_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> ifbdev = dev_priv->fbdev;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4932d298e3af..95c115f6a4c7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1855,7 +1855,7 @@ struct drm_i915_private {
>
> struct drm_i915_gem_object *vlv_pctx;
>
> -#ifdef CONFIG_DRM_I915_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> /* list of fbdev register on this device */
> struct intel_fbdev *fbdev;
> struct work_struct fbdev_suspend_work;
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 16c8052f7eab..9a2f229a1c3a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10131,7 +10131,7 @@ static struct drm_framebuffer *
> mode_fits_in_fbdev(struct drm_device *dev,
> struct drm_display_mode *mode)
> {
> -#ifdef CONFIG_DRM_I915_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_i915_gem_object *obj;
> struct drm_framebuffer *fb;
> @@ -14313,7 +14313,7 @@ intel_user_framebuffer_create(struct drm_device *dev,
> return intel_framebuffer_create(dev, mode_cmd, obj);
> }
>
> -#ifndef CONFIG_DRM_I915_FBDEV
> +#ifndef CONFIG_DRM_FBDEV_EMULATION
> static inline void intel_fbdev_output_poll_changed(struct drm_device *dev)
> {
> }
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
> b/drivers/gpu/drm/i915/intel_dp_mst.c
> index f4fe1183bae6..369f8b6b804f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -406,7 +406,7 @@ static bool intel_dp_mst_get_hw_state(struct
> intel_connector *connector)
>
> static void intel_connector_add_to_fbdev(struct intel_connector *connector)
> {
> -#ifdef CONFIG_DRM_I915_FBDEV
> +#ifdef CONFIG_DRM_FBDEV_EMULATION
> struct drm_i915_private *dev_priv = to_i915(connector->base.dev);
> drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper,
> &connector->base);
> #endif
> @@ -414,7 +414,7 @@ static void
[Intel-gfx] [PATCH 2/2] drm/i915: Commit planes on each crtc separately.
This patch is based on the upstream commit 5ac1c4bcf073ad and amended
for v4.2 to make sure it works as intended.
Repeated calls to begin_crtc_commit can cause warnings like this:
[ 169.127746] BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:616
[ 169.127835] in_atomic(): 0, irqs_disabled(): 1, pid: 1947, name: kms_flip
[ 169.127840] 3 locks held by kms_flip/1947:
[ 169.127843] #0: (&dev->mode_config.mutex){+.+.+.}, at:
[] __drm_modeset_lock_all+0x9c/0x130
[ 169.127860] #1: (crtc_ww_class_acquire){+.+.+.}, at: []
__drm_modeset_lock_all+0xad/0x130
[ 169.127870] #2: (crtc_ww_class_mutex){+.+.+.}, at: []
drm_modeset_lock+0x38/0x110
[ 169.127879] irq event stamp: 665690
[ 169.127882] hardirqs last enabled at (665689): []
_raw_spin_unlock_irqrestore+0x55/0x70
[ 169.127889] hardirqs last disabled at (665690): []
intel_pipe_update_start+0x113/0x5c0 [i915]
[ 169.127936] softirqs last enabled at (665470): []
__do_softirq+0x236/0x650
[ 169.127942] softirqs last disabled at (665465): []
irq_exit+0xc5/0xd0
[ 169.127951] CPU: 1 PID: 1947 Comm: kms_flip Not tainted 4.1.0-rc4-patser+
#4039
[ 169.127954] Hardware name: LENOVO 2349AV8/2349AV8, BIOS G1ETA5WW (2.65 )
04/15/2014
[ 169.127957] 8800c49036f0 8800cde5fa28 817f6907
8001
[ 169.127964] 8800cde5fa58 810aebed
0046
[ 169.127970] 81c5d518 0268
8800cde5fa88
[ 169.127981] Call Trace:
[ 169.127992] [] dump_stack+0x4f/0x7b
[ 169.128001] [] ___might_sleep+0x16d/0x270
[ 169.128008] [] __might_sleep+0x48/0x90
[ 169.128017] [] mutex_lock_nested+0x29/0x410
[ 169.128073] [] ? vgpu_write64+0x220/0x220 [i915]
[ 169.128138] [] ?
ironlake_update_primary_plane+0x2ff/0x410 [i915]
[ 169.128198] [] intel_frontbuffer_flush+0x25/0x70 [i915]
[ 169.128253] [] intel_finish_crtc_commit+0x4c/0x180 [i915]
[ 169.128279] []
drm_atomic_helper_commit_planes+0x12c/0x240 [drm_kms_helper]
[ 169.128338] [] __intel_set_mode+0x684/0x830 [i915]
[ 169.128378] [] intel_crtc_set_config+0x49a/0x620 [i915]
[ 169.128385] [] ? mutex_unlock+0x9/0x10
[ 169.128391] [] drm_mode_set_config_internal+0x69/0x120
[ 169.128398] [] ? might_fault+0x57/0xb0
[ 169.128403] [] drm_mode_setcrtc+0x253/0x620
[ 169.128409] [] drm_ioctl+0x1a0/0x6a0
[ 169.128415] [] ? get_parent_ip+0x11/0x50
[ 169.128424] [] do_vfs_ioctl+0x2f8/0x530
[ 169.128429] [] ? trace_hardirqs_on+0xd/0x10
[ 169.128435] [] ? selinux_file_ioctl+0x56/0x100
[ 169.128439] [] SyS_ioctl+0x81/0xa0
[ 169.128445] [] system_call_fastpath+0x12/0x6f
Solve it by using the newly introduced drm_atomic_helper_commit_planes_on_crtc.
The problem here was that the drm_atomic_helper_commit_planes() helper
we were using was basically designed to do
begin_crtc_commit(crtc #1)
begin_crtc_commit(crtc #2)
...
commit all planes
finish_crtc_commit(crtc #1)
finish_crtc_commit(crtc #2)
The problem here is that since our hardware relies on vblank evasion,
our CRTC 'begin' function waits until we're out of the danger zone in
which register writes might wind up straddling the vblank, then disables
interrupts; our 'finish' function re-enables interrupts after the
registers have been written. The expectation is that the operations between
'begin' and 'end' must be performed without sleeping (since interrupts
are disabled) and should happen as quickly as possible. By clumping all
of the 'begin' calls together, we introducing a couple problems:
* Subsequent 'begin' invocations might sleep (which is illegal)
* The first 'begin' ensured that we were far enough from the vblank that
we could write our registers safely and ensure they all fell within
the same frame. Adding extra delay waiting for subsequent CRTC's
wasn't accounted for and could put us back into the 'danger zone' for
CRTC #1.
This commit solves the problem by using a new helper that allows an
order of operations like:
for each crtc {
begin_crtc_commit(crtc) // sleep (maybe), then disable interrupts
commit planes for this specific CRTC
end_crtc_commit(crtc)// reenable interrupts
}
so that sleeps will only be performed while interrupts are enabled and
we can be sure that registers for a CRTC will be written immediately
once we know we're in the safe zone.
The crtc->config->base.crtc update may seem unrelated, but the helper
will use it to obtain the crtc for the state. Without the update it
will dereference NULL and crash.
Changes since v1:
- Use Matt Roper's commit message.
Signed-off-by: Maarten Lankhorst
Reviewed-by: Matt Roper
Signed-off-by: Jani Nikula
Cc: sta...@vger.kernel.org #v4.2
References: https://bugs.freedesktop.org/show_bug.cgi?id=90398
---
drivers/gpu/drm/i915/intel_atomic.c | 45 +++-
drivers/gpu/drm/i915/intel_display.c | 11 +
2 files changed, 14 insertions(+), 42 delet
[Intel-gfx] [PATCH 1/2] drm/i915: calculate primary visibility changes instead of calling from set_config
This should be much cleaner, with the same effects.
(cherry picked from commit fb9d6cf8c29bfcb0b3c602f7ded87f128d730382)
Signed-off-by: Maarten Lankhorst
Reviewed-by: Matt Roper
Signed-off-by: Jani Nikula
Cc: sta...@vger.kernel.org #v4.2
References: https://bugs.freedesktop.org/show_bug.cgi?id=90398
---
drivers/gpu/drm/i915/intel_display.c | 46 ++--
1 file changed, 7 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 30e0f54ba19d..c2579ded0c36 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12891,20 +12891,11 @@ intel_modeset_stage_output_state(struct drm_device
*dev,
return 0;
}
-static bool primary_plane_visible(struct drm_crtc *crtc)
-{
- struct intel_plane_state *plane_state =
- to_intel_plane_state(crtc->primary->state);
-
- return plane_state->visible;
-}
-
static int intel_crtc_set_config(struct drm_mode_set *set)
{
struct drm_device *dev;
struct drm_atomic_state *state = NULL;
struct intel_crtc_state *pipe_config;
- bool primary_plane_was_visible;
int ret;
BUG_ON(!set);
@@ -12943,38 +12934,8 @@ static int intel_crtc_set_config(struct drm_mode_set
*set)
intel_update_pipe_size(to_intel_crtc(set->crtc));
- primary_plane_was_visible = primary_plane_visible(set->crtc);
-
ret = intel_set_mode_with_config(set->crtc, pipe_config, true);
- if (ret == 0 &&
- pipe_config->base.enable &&
- pipe_config->base.planes_changed &&
- !needs_modeset(&pipe_config->base)) {
- struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
-
- /*
-* We need to make sure the primary plane is re-enabled if it
-* has previously been turned off.
-*/
- if (ret == 0 && !primary_plane_was_visible &&
- primary_plane_visible(set->crtc)) {
- WARN_ON(!intel_crtc->active);
- intel_post_enable_primary(set->crtc);
- }
-
- /*
-* In the fastboot case this may be our only check of the
-* state after boot. It would be better to only do it on
-* the first update, but we don't have a nice way of doing that
-* (and really, set_config isn't used much for high freq page
-* flipping, so increasing its cost here shouldn't be a big
-* deal).
-*/
- if (i915.fastboot && ret == 0)
- intel_modeset_check_state(set->crtc->dev);
- }
-
if (ret) {
DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
set->crtc->base.id, ret);
@@ -13305,6 +13266,9 @@ intel_check_primary_plane(struct drm_plane *plane,
*/
if (IS_BROADWELL(dev))
intel_crtc->atomic.wait_vblank = true;
+
+ if (crtc_state && !needs_modeset(&crtc_state->base))
+ intel_crtc->atomic.post_enable_primary = true;
}
/*
@@ -13317,6 +13281,10 @@ intel_check_primary_plane(struct drm_plane *plane,
if (!state->visible || !fb)
intel_crtc->atomic.disable_ips = true;
+ if (!state->visible && old_state->visible &&
+ crtc_state && !needs_modeset(&crtc_state->base))
+ intel_crtc->atomic.pre_disable_primary = true;
+
intel_crtc->atomic.fb_bits |=
INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Treat foreign dma-buf imports as uncached
On Tue, Aug 11, 2015 at 12:12:08PM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 09:16:46PM +0100, Chris Wilson wrote: > > If the set of pages is being imported from another device, we cannot > > assume that it is fully coherent with the CPU cache, so mark it as such. > > However, if the source is the shared memory vgem allocator, we could > > treat the buffer as being cached (so long as all parties agree in the > > case the same buffer is shared between multiple devices) but as of > > today, vgem cannot export dma-bufs. > > > > Signed-off-by: Chris Wilson > > Cc: Daniel Vetter > > Yay for x86 dma api to refuse acknowleding that incoherent devices exist > (since this should be done in the dma-ap/sg stuff imo for dma-bufs). > > But since x86 assumes that everything is coherent shouldn't we do the > inverse and use snooping/cacheable always? Actually, the convention for PRIME is that transfer bos are uncached because we have to be sure that any write makes it into the foriegn pages, that applies to both GPU and CPU writes, precisely because the target is *incoherent*. > That should be correct for everything modern at least, and for old agp > crap (do we care even, is sharing possible there?) snooping should only > result in a bit more overhead. > > Or where exactly does this blow up? Consider a kms_crc-esque test using a radeon/nouveau bo imported into i915 and accessed using i915 ioctls (with the CRC testing on the radeon scanout). -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Treat foreign dma-buf imports as uncached
On Mon, Aug 10, 2015 at 09:16:46PM +0100, Chris Wilson wrote: > If the set of pages is being imported from another device, we cannot > assume that it is fully coherent with the CPU cache, so mark it as such. > However, if the source is the shared memory vgem allocator, we could > treat the buffer as being cached (so long as all parties agree in the > case the same buffer is shared between multiple devices) but as of > today, vgem cannot export dma-bufs. > > Signed-off-by: Chris Wilson > Cc: Daniel Vetter Yay for x86 dma api to refuse acknowleding that incoherent devices exist (since this should be done in the dma-ap/sg stuff imo for dma-bufs). But since x86 assumes that everything is coherent shouldn't we do the inverse and use snooping/cacheable always? That should be correct for everything modern at least, and for old agp crap (do we care even, is sharing possible there?) snooping should only result in a bit more overhead. Or where exactly does this blow up? -Daniel > --- > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 14 ++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c > b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > index 7748d57adffd..da5e4fb02350 100644 > --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c > +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > @@ -265,8 +265,22 @@ struct drm_gem_object *i915_gem_prime_import(struct > drm_device *dev, > i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops); > obj->base.import_attach = attach; > > + /* This bo is imported from a foreign HW device with which we cannot > + * assume any coherency. Note that if this dma-buf was imported from > + * a simple page allocator, like vgem, then we could treat this as > + * cacheable (so long as all parties agree on that convention - i.e. > + * if the same bo was shared with nouveau/radeon they also treat it > + * as CPU cacheeable so that coherency is maintained between all > + * parties). > + */ > + ret = i915_gem_object_set_cache_level(obj, I915_CACHE_NONE); > + if (ret) > + goto fail_obj; > + > return &obj->base; > > +fail_obj: > + drm_gem_object_unreference(&obj->base); > fail_detach: > dma_buf_detach(dma_buf, attach); > dma_buf_put(dma_buf); > -- > 2.5.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/6] drm/i915: eDP can be present on DDI-E
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Tuesday, August 11, 2015 5:47 PM
> To: Zhang, Xiong Y
> Cc: intel-gfx@lists.freedesktop.org; Vivi, Rodrigo
> Subject: Re: [Intel-gfx] [PATCH 4/6] drm/i915: eDP can be present on DDI-E
>
> On Thu, Aug 06, 2015 at 03:51:39PM +0800, Xiong Zhang wrote:
> > From: Rodrigo Vivi
> >
> > On Skylake we have eDP-to-VGA using DDI-E and another aux.
> > So let's identify it properly.
>
> eDP means panel (the only difference in the code we have between eDP and
> DP is the power panel sequncing). VGA very much means no panel.
>
> Is this some impressive hack (dp->vga dongle using panel power as it's power
> source) or what's going on here? Or just confused commit message?
[Zhang, Xiong Y] Sorry, it's a dp-to-vga converter connected to DDI-E.
DDI-E has the same role as DDI-B/C/D, so eDP could connect to DDI-E also.
>
> I'll punt on this for now.
> -Daniel
>
> >
> > Also let's remove duplicated definitions to avoid later confusion.
> >
> > Signed-off-by: Rodrigo Vivi
> > ---
> > drivers/gpu/drm/i915/intel_bios.h | 5 -
> > drivers/gpu/drm/i915/intel_dp.c | 9 +
> > 2 files changed, 5 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_bios.h
> > b/drivers/gpu/drm/i915/intel_bios.h
> > index 02255d8..a2ef0df 100644
> > --- a/drivers/gpu/drm/i915/intel_bios.h
> > +++ b/drivers/gpu/drm/i915/intel_bios.h
> > @@ -747,11 +747,6 @@ int intel_parse_bios(struct drm_device *dev);
> > #defineDVO_C 2
> > #defineDVO_D 3
> >
> > -/* define the PORT for DP output type */
> > -#definePORT_IDPB 7
> > -#definePORT_IDPC 8
> > -#definePORT_IDPD 9
> > -
> > /* Possible values for the "DVO Port" field for versions >= 155: */
> > #define DVO_PORT_HDMIA 0
> > #define DVO_PORT_HDMIB 1
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 7cd47bc..0643a91 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4978,16 +4978,17 @@ intel_trans_dp_port_sel(struct drm_crtc *crtc)
> > return -1;
> > }
> >
> > -/* check the VBT to see whether the eDP is on DP-D port */
> > +/* check the VBT to see whether the eDP is on another port */
> > bool intel_dp_is_edp(struct drm_device *dev, enum port port) {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > union child_device_config *p_child;
> > int i;
> > static const short port_mapping[] = {
> > - [PORT_B] = PORT_IDPB,
> > - [PORT_C] = PORT_IDPC,
> > - [PORT_D] = PORT_IDPD,
> > + [PORT_B] = DVO_PORT_DPB,
> > + [PORT_C] = DVO_PORT_DPC,
> > + [PORT_D] = DVO_PORT_DPD,
> > + [PORT_E] = DVO_PORT_DPE,
> > };
> >
> > if (port == PORT_A)
> > --
> > 2.1.4
> >
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/atomic: Fix bookkeeping with TEST_ONLY.
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang...@intel.com) Task id: 6996 -Summary- Platform Delta drm-intel-nightly Series Applied ILK -2 302/302 300/302 SNB 315/315 315/315 IVB 336/336 336/336 BYT 283/283 283/283 HSW 378/378 378/378 -Detailed- Platform Testdrm-intel-nightly Series Applied *ILK igt@kms_flip@flip-vs-dpms-interruptible PASS(1) DMESG_WARN(1) *ILK igt@kms_flip@wf_vblank-vs-modeset-interruptible PASS(1) DMESG_WARN(1) Note: You need to pay more attention to line start with '*' ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Check idle to active before processing CSQ
On Fri, Aug 07, 2015 at 03:30:12PM +0100, Siluvery, Arun wrote: > On 07/08/2015 12:52, Daniel Vetter wrote: > >On Fri, Aug 07, 2015 at 11:15:56AM +0300, Mika Kuoppala wrote: > >>Daniel Vetter writes: > >> > >>>On Thu, Aug 06, 2015 at 05:09:17PM +0300, Mika Kuoppala wrote: > If idle to active bit is set, the rest of the fields > in CSQ are not valid. > > Bail out early if this is the case in order to prevent > rest of the loop inspecting stale values. > > Signed-off-by: Mika Kuoppala > >>> > > looks good to me, didn't observe any impact with this patch. > Reviewed-by: Arun Siluvery Queued for -next, thanks for the patch. -Daniel > > regards > Arun > > >>>Same questions here too, what's the impact. E.g. if you only found this by > >>>bspec/code inspection then it's for -next, but if it's to fix some known > >>>breakage then it's for -fixes + cc: stable. > >>> > >> > >>To this and the masked write one: Both of these were found > >>when I was trying to find out root cause for skl hangs. > >> > >>They are both for -next. Both are in the correctness > >>department vrt bspec and I haven't observed any other > >>impact. > >> > >>Point taken on being more verbose. > > > >Thanks I added a note about this to the first patch and merged it. This > >one here still seems to miss an r-b. > >-Daniel > > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Spam less on dp aux send/receive problems
On Mon, Aug 10, 2015 at 02:01:56PM +0300, Jani Nikula wrote:
> On Mon, 10 Aug 2015, Jani Nikula wrote:
> > On Thu, 06 Aug 2015, Mika Kuoppala wrote:
> >> If we encounter frequent problems with dp aux channel
> >> communications, we end up spamming the dmesg with the
> >> exact similar trace and status.
> >>
> >> Inject a new backtrace only if we have new information
> >> to share as otherwise we flush out all other important
> >> stuff.
> >>
> >> Signed-off-by: Mika Kuoppala
> >> ---
> >> drivers/gpu/drm/i915/intel_dp.c | 11 +--
> >> 1 file changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> >> b/drivers/gpu/drm/i915/intel_dp.c
> >> index df7e2cf..6b2f7af 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -849,8 +849,15 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
> >>}
> >>
> >>if (try == 3) {
> >> - WARN(1, "dp_aux_ch not started status 0x%08x\n",
> >> - I915_READ(ch_ctl));
> >> + static u32 last_status = -1;
> >> + const u32 status = I915_READ(ch_ctl);
> >> +
> >> + if (status != last_status) {
> >> + WARN(1, "dp_aux_ch not started status 0x%08x\n",
> >> + status);
> >> + last_status = status;
> >> + }
> >> +
> >
> > But now you'll also skip the logging even if there's been a day and a
> > million successful transfers since the last error... I understand your
> > concern, but if you feel you must do something like this, please at
> > least reset last_status on success.
>
> Hmh, I see now that this has already been merged... :(
Well I like the idea, Mika please supply a fixup patch.
-Daniel
>
>
> >
> > BR,
> > Jani.
> >
> >
> >>ret = -EBUSY;
> >>goto out;
> >>}
> >> --
> >> 2.1.4
> >>
> >> ___
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Jani Nikula, Intel Open Source Technology Center
>
> --
> Jani Nikula, Intel Open Source Technology Center
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 6/6] drm/i915: Enable HDMI on DDI-E
On Thu, Aug 06, 2015 at 03:51:41PM +0800, Xiong Zhang wrote:
> DDI-E doesn't have the correspondent GMBUS pin.
>
> We rely on VBT to tell us which one it being used instead.
>
> The DVI/HDMI on shared port couldn't exist.
>
> This patch isn't tested without hardware wchich has HDMI
> on DDI-E.
>
> Signed-off-by: Xiong Zhang
> ---
> drivers/gpu/drm/i915/i915_drv.h | 5 +
> drivers/gpu/drm/i915/intel_bios.c | 28
> drivers/gpu/drm/i915/intel_hdmi.c | 18 ++
> 3 files changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 6d93334..5d8e7fe 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1414,6 +1414,10 @@ enum modeset_restore {
> #define DP_AUX_C 0x20
> #define DP_AUX_D 0x30
>
> +#define DDC_PIN_B 0x05
> +#define DDC_PIN_C 0x04
> +#define DDC_PIN_D 0x06
> +
> struct ddi_vbt_port_info {
> /*
>* This is an index in the HDMI/DVI DDI buffer translation table.
> @@ -1428,6 +1432,7 @@ struct ddi_vbt_port_info {
> uint8_t supports_dp:1;
>
> uint8_t alternate_aux_channel;
> + uint8_t alternate_ddc_pin;
> };
>
> enum psr_lines_to_wait {
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index 16cdf17..265227f 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -894,7 +894,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> uint8_t hdmi_level_shift;
> int i, j;
> bool is_dvi, is_hdmi, is_dp, is_edp, is_crt;
> - uint8_t aux_channel;
> + uint8_t aux_channel, ddc_pin;
> /* Each DDI port can have more than one value on the "DVO Port" field,
>* so look for all the possible values for each port and abort if more
>* than one is found. */
> @@ -928,6 +928,7 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> return;
>
> aux_channel = child->raw[25];
> + ddc_pin = child->common.ddc_pin;
>
> is_dvi = child->common.device_type & DEVICE_TYPE_TMDS_DVI_SIGNALING;
> is_dp = child->common.device_type & DEVICE_TYPE_DISPLAYPORT_OUTPUT;
> @@ -959,11 +960,30 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> DRM_DEBUG_KMS("Port %c is internal DP\n", port_name(port));
>
> if (is_dvi) {
> - if (child->common.ddc_pin == 0x05 && port != PORT_B)
> + if (port == PORT_E) {
> + info->alternate_ddc_pin = ddc_pin;
> + /* if DDIE share ddc pin with other port, then
> + * dvi/hdmi couldn't exist on the shared port.
> + * Otherwise they share the same ddc bin and system
> + * couldn't communicate with them seperately. */
> + if (ddc_pin == DDC_PIN_B) {
> +
> dev_priv->vbt.ddi_port_info[PORT_B].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_B].supports_hdmi = 0;
> + }
> + else if (ddc_pin == DDC_PIN_C) {
> +
> dev_priv->vbt.ddi_port_info[PORT_C].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_C].supports_hdmi = 0;
> + }
> + else if (ddc_pin == DDC_PIN_D) {
> +
> dev_priv->vbt.ddi_port_info[PORT_D].supports_dvi = 0;
> +
> dev_priv->vbt.ddi_port_info[PORT_D].supports_hdmi = 0;
> + }
> + }
> + else if (ddc_pin == DDC_PIN_B && port != PORT_B)
> DRM_DEBUG_KMS("Unexpected DDC pin for port B\n");
> - if (child->common.ddc_pin == 0x04 && port != PORT_C)
> + else if (ddc_pin == DDC_PIN_C && port != PORT_C)
> DRM_DEBUG_KMS("Unexpected DDC pin for port C\n");
> - if (child->common.ddc_pin == 0x06 && port != PORT_D)
> + else if (ddc_pin == DDC_PIN_D && port != PORT_D)
> DRM_DEBUG_KMS("Unexpected DDC pin for port D\n");
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 70bad5b..e1f6e81 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1991,6 +1991,24 @@ void intel_hdmi_init_connector(struct
> intel_digital_port *intel_dig_port,
> intel_hdmi->ddc_bus = GMBUS_PIN_DPD;
> intel_encoder->hpd_pin = HPD_PORT_D;
> break;
> + case PORT_E:
> + /* On SKL PORT E doesn't have seperate GMBUS pin
> + * We rely on VBT to set a proper alternate GMBUS pin. */
> + switch (dev_priv->vbt.ddi_port_info[
Re: [Intel-gfx] [PATCH 4/6] drm/i915: eDP can be present on DDI-E
On Thu, Aug 06, 2015 at 03:51:39PM +0800, Xiong Zhang wrote:
> From: Rodrigo Vivi
>
> On Skylake we have eDP-to-VGA using DDI-E and another aux.
> So let's identify it properly.
eDP means panel (the only difference in the code we have between eDP and
DP is the power panel sequncing). VGA very much means no panel.
Is this some impressive hack (dp->vga dongle using panel power as it's
power source) or what's going on here? Or just confused commit message?
I'll punt on this for now.
-Daniel
>
> Also let's remove duplicated definitions to avoid later
> confusion.
>
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_bios.h | 5 -
> drivers/gpu/drm/i915/intel_dp.c | 9 +
> 2 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 02255d8..a2ef0df 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -747,11 +747,6 @@ int intel_parse_bios(struct drm_device *dev);
> #define DVO_C 2
> #define DVO_D 3
>
> -/* define the PORT for DP output type */
> -#define PORT_IDPB 7
> -#define PORT_IDPC 8
> -#define PORT_IDPD 9
> -
> /* Possible values for the "DVO Port" field for versions >= 155: */
> #define DVO_PORT_HDMIA 0
> #define DVO_PORT_HDMIB 1
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 7cd47bc..0643a91 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4978,16 +4978,17 @@ intel_trans_dp_port_sel(struct drm_crtc *crtc)
> return -1;
> }
>
> -/* check the VBT to see whether the eDP is on DP-D port */
> +/* check the VBT to see whether the eDP is on another port */
> bool intel_dp_is_edp(struct drm_device *dev, enum port port)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> union child_device_config *p_child;
> int i;
> static const short port_mapping[] = {
> - [PORT_B] = PORT_IDPB,
> - [PORT_C] = PORT_IDPC,
> - [PORT_D] = PORT_IDPD,
> + [PORT_B] = DVO_PORT_DPB,
> + [PORT_C] = DVO_PORT_DPC,
> + [PORT_D] = DVO_PORT_DPD,
> + [PORT_E] = DVO_PORT_DPE,
> };
>
> if (port == PORT_A)
> --
> 2.1.4
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5
On Tue, Aug 11, 2015 at 11:30:48AM +0200, Daniel Vetter wrote: > On Mon, Aug 10, 2015 at 11:15:30AM +0100, Chris Wilson wrote: > > On Mon, Aug 03, 2015 at 01:09:11PM -0700, Jesse Barnes wrote: > > > Looks like > > > > > > commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6 > > > Author: Maarten Lankhorst > > > Date: Mon Jun 15 12:33:53 2015 +0200 > > > drm/i915: Update less state during modeset. > > > > > > introduced the unconditional calling of disable_shared_dpll, but didn't > > > fix up pre-gen5 to avoid the BUG_ON at the top of the function. > > > > > > So change the BUG_ON into a gen check (alternately we could move the > > > BUG_ON until later, since we shouldn't have a pll struct here either, > > > but this seems clearer to read). > > > > > > This fixes a crash on load on my x200s platform. > > > > > > Signed-off-by: Jesse Barnes > > > > This blows up in 4.1 (a BUG_ON during boot causing a hard lockup). > > The reference offending commit is 4.3 only. Are you sure it's the same > BUG_ON? Which machine? It's entirely probably that I messed up my bisect, something about compiling a kernel a day for a couple of weeks and forgetting which step it is at... -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] HDMI optimization series
On Mon, Aug 10, 2015 at 02:05:47PM +0530, Jindal, Sonika wrote: > > > On 8/10/2015 1:38 PM, Daniel Vetter wrote: > >On Mon, Aug 10, 2015 at 04:50:37AM +, Jindal, Sonika wrote: > >>Hi Daniel, > >> > >>That patch was already merged: > >>http://lists.freedesktop.org/archives/intel-gfx/2015-July/071142.html > >> > >>For SKL, the above patch helped in getting the correct ISR bits set. > >>One option is to enable the HDMI optimization from VLV onwards. > >>I don't have an ivb machine to try out the issue. > > > >ivb is simple the machine I have here, but when we tried this the last > >time around we had reports for all platforms (your patch still doesn't > >cite the relevant sha1 btw). I think there are 3 possible explanations: > > > Yes, I don't know which were those patches and how to find them.. git blame (recursively) and git log on intel_hdmi.c will tell you the story. I require all these extensive git commit messages because I dig them out daily. Also git log --pickaxe (well I use the equivalent in the gitk gui). If these tools are generally unknown in the vpg display team then we absolutely need to do a training session about them, they're extremely powerful and useful to dig out the history of the i915 code. > >a) we do something wrong with hpd handling on these platforms. That seems > >to be the explanation you favour (with the gen >= 7 checks and all that), > >but I think it's very unlikely: On each platform where we had reports of > >hpd being broken there was also machines where hpd works perfectly fine. > > > Not sure, I will find one ivb system and try on that. > > >b) There's broken HDMI (or DVI) sinks out there. If that's the case we can > >never merge your patch. > I doubt this because we have tested these patches with many sinks in the > past with VLV/CHV. > > > >c) There's something in vbt or somewhere else that tells the windows > >driver whether using hpd or not is ok (and the hpd problem is actually an > >issue with certain OEM machines ...). > > > No, nothing like that. > > >I hoped that with your hpd handling fix we'd have some indication that our > >hpd troubles are of type a). But since I tested with your patch that > >didn't work out. > > > >And until we have some evidence that our hpd troubles aren't type b) I > >really don't want to merge any patch which relies upon hpd bits for hdmi. > >-Daniel > > > I will try on ivb. I don't think it's ivb specific, since we do have ivb machines where hpd works perfectly fine. Same for every other platform. The only thing which seems common is that DP+ connectors work, but some of the hdmi-only connectors fail. That's why I wondered whether there's something i915 does different than the windows driver. In case you can get hold of one, the broken laptop I have here is an Asus UX31A with ivb. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: fix stolen bios_reserved checks
On Mon, Aug 10, 2015 at 07:06:44PM +0100, Chris Wilson wrote: > On Mon, Aug 10, 2015 at 02:57:32PM -0300, Paulo Zanoni wrote: > > I started digging this when I noticed that the BDW code was just > > reserving 1mb by coincidence since it was reading reserved fields. > > Then I noticed we didn't have any values set for SNB and earlier, and > > that the HSW sizes were wrong. After that, I noticed that the reserved > > area has a specific start, and may not exactly end where the stolen > > memory ends. I also noticed the base pointer can be zero. So I decided > > to just write a single patch fixing everything instead of 20 patches > > that would be much harder to review. > > > > This patch may solve random stolen memory corruption/problems on > > almost all platforms. Notice that since this is always dealing with > > the top of the stolen memory, the problems are not so easy to > > reproduce - especially since FBC is still disabled by default. > > > > One of the major differences of this patch is that we now look at both > > the size and base address. By only looking at the size we were > > assuming that the reserved area was always at the very top of > > stolen, which is not always true. > > > > After we merge the patch series that allows user space to allocate > > stolen memory we'll be able to write IGT tests that maybe catch the > > bugs fixed by this patch. > > > > v2: > > - s/BIOS reserved/stolen reserved/g (Chris) > > - Don't DRM_ERROR if we can't do anything about it (Chris) > > - Improve debug messages (Chris). > > - Use the gen7 version instead of gen6 on HSW. Tom found some > > documentation problems, so I think with gen7 we're on the safer > > side (Tom). > > > > Signed-off-by: Paulo Zanoni > > Looks ok to me, I'd push for DRM_INFO() for the amount of memory > available (since I think that is interesting as a user). > > Reviewed-by: Chris Wilson Queued for -next, thanks for the patch. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5
On Mon, Aug 10, 2015 at 11:15:30AM +0100, Chris Wilson wrote: > On Mon, Aug 03, 2015 at 01:09:11PM -0700, Jesse Barnes wrote: > > Looks like > > > > commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6 > > Author: Maarten Lankhorst > > Date: Mon Jun 15 12:33:53 2015 +0200 > > drm/i915: Update less state during modeset. > > > > introduced the unconditional calling of disable_shared_dpll, but didn't > > fix up pre-gen5 to avoid the BUG_ON at the top of the function. > > > > So change the BUG_ON into a gen check (alternately we could move the > > BUG_ON until later, since we shouldn't have a pll struct here either, > > but this seems clearer to read). > > > > This fixes a crash on load on my x200s platform. > > > > Signed-off-by: Jesse Barnes > > This blows up in 4.1 (a BUG_ON during boot causing a hard lockup). The reference offending commit is 4.3 only. Are you sure it's the same BUG_ON? Which machine? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/dp/mst: Remove port after removing connector.
On Tue, 11 Aug 2015, Daniel Vetter wrote:
> On Tue, Aug 11, 2015 at 09:54:29AM +0200, Maarten Lankhorst wrote:
>> The port is removed synchronously, but the connector delayed.
>> This causes a use after free which can cause a kernel BUG with
>> slug_debug=FPZU. This is fixed by freeing the port after the
>> connector.
>>
>> This fixes a regression introduced with
>> 6b8eeca65b18ae77e175cc2b6571731f0ee413bf
>> "drm/dp/mst: close deadlock in connector destruction."
>>
>> Cc: sta...@vger.kernel.org
>> Cc: Dave Airlie
>> Signed-off-by: Maarten Lankhorst
>
> Reviewed-by: Daniel Vetter
>
> Jani, can you please pick this up for topic/drm-fixes since Dave's still
> on vacation this week?
Done.
BR,
Jani.
> -Daniel
>
>> ---
>> drivers/gpu/drm/drm_dp_mst_topology.c | 19 +--
>> include/drm/drm_crtc.h| 2 --
>> 2 files changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index b0487c9f018c..eb603f1defc2 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -873,9 +873,10 @@ static void drm_dp_destroy_port(struct kref *kref)
>> from an EDID retrieval */
>> if (port->connector) {
>> mutex_lock(&mgr->destroy_connector_lock);
>> -list_add(&port->connector->destroy_list,
>> &mgr->destroy_connector_list);
>> +list_add(&port->next, &mgr->destroy_connector_list);
>> mutex_unlock(&mgr->destroy_connector_lock);
>> schedule_work(&mgr->destroy_connector_work);
>> +return;
>> }
>> drm_dp_port_teardown_pdt(port, port->pdt);
>>
>> @@ -2659,7 +2660,7 @@ static void drm_dp_tx_work(struct work_struct *work)
>> static void drm_dp_destroy_connector_work(struct work_struct *work)
>> {
>> struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
>> drm_dp_mst_topology_mgr, destroy_connector_work);
>> -struct drm_connector *connector;
>> +struct drm_dp_mst_port *port;
>>
>> /*
>> * Not a regular list traverse as we have to drop the destroy
>> @@ -2668,15 +2669,21 @@ static void drm_dp_destroy_connector_work(struct
>> work_struct *work)
>> */
>> for (;;) {
>> mutex_lock(&mgr->destroy_connector_lock);
>> -connector =
>> list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_connector,
>> destroy_list);
>> -if (!connector) {
>> +port = list_first_entry_or_null(&mgr->destroy_connector_list,
>> struct drm_dp_mst_port, next);
>> +if (!port) {
>> mutex_unlock(&mgr->destroy_connector_lock);
>> break;
>> }
>> -list_del(&connector->destroy_list);
>> +list_del(&port->next);
>> mutex_unlock(&mgr->destroy_connector_lock);
>>
>> -mgr->cbs->destroy_connector(mgr, connector);
>> +mgr->cbs->destroy_connector(mgr, port->connector);
>> +
>> +drm_dp_port_teardown_pdt(port, port->pdt);
>> +
>> +if (!port->input && port->vcpi.vcpi > 0)
>> +drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
>> +kfree(port);
>> }
>> }
>>
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index 574656965126..373b1bc6de96 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -745,8 +745,6 @@ struct drm_connector {
>> uint8_t num_h_tile, num_v_tile;
>> uint8_t tile_h_loc, tile_v_loc;
>> uint16_t tile_h_size, tile_v_size;
>> -
>> -struct list_head destroy_list;
>> };
>>
>> /**
>> --
>> 2.1.0
>>
>> ___
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Update HAS_PSR macro to include all gen>=8 platforms
On Tue, Aug 11, 2015 at 04:30:10AM +, Jindal, Sonika wrote:
> I replied to Daniel last time. Pasting it on mailing list as well:
>
> "Yes this is tested on android with HW tracking. Not sure about enabling
> by default part. But this patch will be anyways required.
I'd like it to be tested with igt testcases from Paulo/Rodrigo though
since that's what we use for upstream validation and to make sure all
upstream use-cases are working too. Otherwise we just make an accounting
trick and shift the maintainenance burden from android to upstream without
being able to use the code really. And looking at psr fixing up the last
10% took 90% of all the effort.
And if there are failures still in the current igts (both kms_psr and
kms_frontbuffer_tracking) then I want a plan for how to get this all
addressed.
-Daniel
>
> Regards,
> Sonika"
>
> -Original Message-
> From: Daniel Vetter [mailto:daniel.vet...@ffwll.ch] On Behalf Of Daniel Vetter
> Sent: Tuesday, July 21, 2015 3:18 PM
> To: Lespiau, Damien
> Cc: Jindal, Sonika; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Update HAS_PSR macro to include
> all gen>=8 platforms
>
> On Tue, Jul 21, 2015 at 10:31:19AM +0100, Damien Lespiau wrote:
> > On Tue, Jul 21, 2015 at 02:48:31PM +0530, Sonika Jindal wrote:
> > > This is to get PSR support for bxt.
> > >
> > > Signed-off-by: Sonika Jindal
> >
> > Maybe with a drm/i915/bxt prefix:
> >
> > Reviewed-by: Damien Lespiau
>
> Is this actually tested? Can we maybe enable psr by default (Rodrigo seems so
> close ...)?
> -Daniel
>
> >
> > --
> > Damien
> >
> > > ---
> > > drivers/gpu/drm/i915/i915_drv.h |5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > > b/drivers/gpu/drm/i915/i915_drv.h index 718170c..54d2729 100644
> > > --- a/drivers/gpu/drm/i915/i915_drv.h
> > > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > > @@ -2537,9 +2537,8 @@ struct drm_i915_cmd_table {
> > >
> > > #define HAS_DDI(dev) (INTEL_INFO(dev)->has_ddi)
> > > #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> > > -#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_BROADWELL(dev)
> > > || \
> > > - IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev) || \
> > > - IS_SKYLAKE(dev))
> > > +#define HAS_PSR(dev) (IS_HASWELL(dev) || IS_VALLEYVIEW(dev)
> > > || \
> > > + INTEL_INFO(dev)->gen >= 8)
> > > #define HAS_RUNTIME_PM(dev) (IS_GEN6(dev) || IS_HASWELL(dev) || \
> > >IS_BROADWELL(dev) || IS_VALLEYVIEW(dev) || \
> > >IS_SKYLAKE(dev))
> > > --
> > > 1.7.10.4
> > >
> > > ___
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > ___
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/dp/mst: Remove port after removing connector.
On Tue, Aug 11, 2015 at 09:54:29AM +0200, Maarten Lankhorst wrote:
> The port is removed synchronously, but the connector delayed.
> This causes a use after free which can cause a kernel BUG with
> slug_debug=FPZU. This is fixed by freeing the port after the
> connector.
>
> This fixes a regression introduced with
> 6b8eeca65b18ae77e175cc2b6571731f0ee413bf
> "drm/dp/mst: close deadlock in connector destruction."
>
> Cc: sta...@vger.kernel.org
> Cc: Dave Airlie
> Signed-off-by: Maarten Lankhorst
Reviewed-by: Daniel Vetter
Jani, can you please pick this up for topic/drm-fixes since Dave's still
on vacation this week?
-Daniel
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 19 +--
> include/drm/drm_crtc.h| 2 --
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b0487c9f018c..eb603f1defc2 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -873,9 +873,10 @@ static void drm_dp_destroy_port(struct kref *kref)
> from an EDID retrieval */
> if (port->connector) {
> mutex_lock(&mgr->destroy_connector_lock);
> - list_add(&port->connector->destroy_list,
> &mgr->destroy_connector_list);
> + list_add(&port->next, &mgr->destroy_connector_list);
> mutex_unlock(&mgr->destroy_connector_lock);
> schedule_work(&mgr->destroy_connector_work);
> + return;
> }
> drm_dp_port_teardown_pdt(port, port->pdt);
>
> @@ -2659,7 +2660,7 @@ static void drm_dp_tx_work(struct work_struct *work)
> static void drm_dp_destroy_connector_work(struct work_struct *work)
> {
> struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
> drm_dp_mst_topology_mgr, destroy_connector_work);
> - struct drm_connector *connector;
> + struct drm_dp_mst_port *port;
>
> /*
>* Not a regular list traverse as we have to drop the destroy
> @@ -2668,15 +2669,21 @@ static void drm_dp_destroy_connector_work(struct
> work_struct *work)
>*/
> for (;;) {
> mutex_lock(&mgr->destroy_connector_lock);
> - connector =
> list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_connector,
> destroy_list);
> - if (!connector) {
> + port = list_first_entry_or_null(&mgr->destroy_connector_list,
> struct drm_dp_mst_port, next);
> + if (!port) {
> mutex_unlock(&mgr->destroy_connector_lock);
> break;
> }
> - list_del(&connector->destroy_list);
> + list_del(&port->next);
> mutex_unlock(&mgr->destroy_connector_lock);
>
> - mgr->cbs->destroy_connector(mgr, connector);
> + mgr->cbs->destroy_connector(mgr, port->connector);
> +
> + drm_dp_port_teardown_pdt(port, port->pdt);
> +
> + if (!port->input && port->vcpi.vcpi > 0)
> + drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
> + kfree(port);
> }
> }
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 574656965126..373b1bc6de96 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -745,8 +745,6 @@ struct drm_connector {
> uint8_t num_h_tile, num_v_tile;
> uint8_t tile_h_loc, tile_v_loc;
> uint16_t tile_h_size, tile_v_size;
> -
> - struct list_head destroy_list;
> };
>
> /**
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/core: Set mode to NULL when connectors in a set drops to 0.
On Tue, Aug 11, 2015 at 09:54:59AM +0200, Maarten Lankhorst wrote:
> Without this when a MST connector is removed drm_atomic_helper_set_config
> can complain about set->mode && !set->num_connectors.
>
> [ cut here ]
> WARNING: CPU: 2 PID: 2403 at drivers/gpu/drm/drm_atomic_helper.c:1673
> drm_atomic_helper_set_config+0x22e/0x420()
> CPU: 2 PID: 2403 Comm: kms_flip Not tainted 4.2.0-rc5 #4233
> Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
> 81ac75e8 88004e4ffbf8 81714c34 8000
> 88004e4ffc38 8107bf81 88004e4ffc48
> 8800d8ca0690 8800d8d7a080 8800d8cc2290 8800d07bc9f0
> Call Trace:
> [] dump_stack+0x4f/0x7b
> [] warn_slowpath_common+0x81/0xc0
> [] warn_slowpath_null+0x15/0x20
> [] drm_atomic_helper_set_config+0x22e/0x420
> [] ? drm_atomic_helper_plane_set_property+0x84/0xc0
> [] drm_mode_set_config_internal+0x61/0x100
> [] restore_fbdev_mode+0xbd/0xe0
> [] drm_fb_helper_restore_fbdev_mode_unlocked+0x24/0x70
> [] intel_fbdev_restore_mode+0x21/0x80 [i915]
> [] i915_driver_lastclose+0x9/0x10 [i915]
> [] drm_lastclose+0x29/0x130
> [] drm_release+0x314/0x500
> [] __fput+0xe5/0x1f0
> [] fput+0x9/0x10
> [] task_work_run+0x88/0xb0
> [] do_exit+0x37f/0xa90
> [] ? selinux_file_ioctl+0x48/0xc0
> [] ? security_file_ioctl+0x3e/0x60
> [] do_group_exit+0x40/0xa0
> [] SyS_exit_group+0xf/0x10
> [] entry_SYSCALL_64_fastpath+0x12/0x6a
> ---[ end trace 0daf358c49351567 ]---
>
> Signed-off-by: Maarten Lankhorst
Applied to drm-misc, thanks.
-Daniel
> ---
> drivers/gpu/drm/drm_fb_helper.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5875059a7625..418d299f3b12 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -168,11 +168,14 @@ static void remove_from_modeset(struct drm_mode_set
> *set,
> }
> set->num_connectors--;
>
> - /* because i915 is pissy about this..
> + /*
>* TODO maybe need to makes sure we set it back to !=NULL somewhere?
>*/
> - if (set->num_connectors == 0)
> + if (set->num_connectors == 0) {
> set->fb = NULL;
> + drm_mode_destroy(connector->dev, set->mode);
> + set->mode = NULL;
> + }
> }
>
> int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
> --
> 2.1.0
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
On Tue, 11 Aug 2015, "Yang, Libin" wrote:
> Hi Jani,
>
>> -Original Message-
>> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>> Sent: Tuesday, August 11, 2015 3:14 PM
>> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
>> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
>> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
>> modeset
>>
>> On Tue, 11 Aug 2015, "Yang, Libin" wrote:
>> > Hi Jani,
>> >
>> > Thanks for careful reviewing for all the patches, please
>> > see my comments.
>> >
>> >> -Original Message-
>> >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>> >> Sent: Monday, August 10, 2015 8:10 PM
>> >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
>> >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
>> >> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
>> >> modeset
>> >>
>> >> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
>> >> > From: Libin Yang
>> >> >
>> >> > When modeset occurs and the TMDS frequency is set to some
>> >> > speical value, the N/CTS need to be set manually if audio
>> >> > is playing.
>> >>
>> >> When modeset occurs, we disable everything, and then re-enable
>> >> everything, and notify the audio driver every step of the way. IIUC
>> >> there should be no audio playing across the modeset, and when the
>> >> modeset is complete and we've set the valid ELD, the audio driver
>> >> should
>> >> call the callback from the earlier patches to set the correct audio
>> >> rate.
>> >>
>> >> Why is this patch needed? Please enlighten me.
>> >
>> > Please image this scenario: when audio is playing, the gfx driver
>> > does the modeset. In this situation, after modeset, audio driver
>> > will do nothing and continue playing. And audio driver will not call
>> > set_ncts.
>>
>> Why does the audio driver not do anything here? Whenever there's a
>> modeset, the graphics driver will disable audio and invalidate the ELD,
>> which should cause an interrupt to notify the audio driver about
>> this. This is no different from a hot-unplug, in fact I can't see how
>> the audio driver could even detect whether there's been a hotplug or
>> not
>> when there's a codec disable/enable.
>
> Yes, it will trigger an interrupt to audio driver when unplug
> and another interrupt for hotplug. But from audio driver,
> we will not stop the playing and resume the playing based
> on the hotplug or modeset. The audio is always playing during
> the hotplug/modeset.
But the whole display, and consequently ELD, might have changed during
the hotplug, why do you assume you can just keep playing?! The display
might even have changed from HDMI to DP or vice versa.
We've also been putting a lot of effort into not depending on previous
state when enabling the outputs, for more reliability. We generally
don't do partial changes, we disable everything and then enable
again. And now you're suggesting we look at some register which for all
we know may have been valid for some other display?
Timeout.
BR,
Jani.
>
>>
>> BR,
>> Jani.
>>
>>
>> >
>> >>
>> >> Also some comments in-line, provided you've convinced me first. ;)
>> >>
>> >> > Signed-off-by: Libin Yang
>> >> > ---
>> >> > drivers/gpu/drm/i915/i915_reg.h| 6 ++
>> >> > drivers/gpu/drm/i915/intel_audio.c | 42
>> >> ++
>> >> > 2 files changed, 48 insertions(+)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >> b/drivers/gpu/drm/i915/i915_reg.h
>> >> > index da2d128..85f3beb 100644
>> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> >> > @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
>> >> > _HSW_AUD_DIG_CNVT_2)
>> >> > #define DIP_PORT_SEL_MASK 0x3
>> >> >
>> >> > +#define _HSW_AUD_STR_DESC_10x65084
>> >> > +#define _HSW_AUD_STR_DESC_20x65184
>> >> > +#define AUD_STR_DESC(pipe) _PIPE(pipe, \
>> >> > +_HSW_AUD_STR_DESC_1,
>> >> \
>> >> > +_HSW_AUD_STR_DESC_2)
>> >> > +
>> >> > #define _HSW_AUD_EDID_DATA_A 0x65050
>> >> > #define _HSW_AUD_EDID_DATA_B 0x65150
>> >> > #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
>> >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> >> b/drivers/gpu/drm/i915/intel_audio.c
>> >> > index eddf37f..082f96d 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> >> > @@ -235,6 +235,9 @@ static void
>> hsw_audio_codec_enable(struct
>> >> drm_connector *connector,
>> >> > const uint8_t *eld = connector->eld;
>> >> > uint32_t tmp;
>> >> > int len, i;
>> >> > + int cvt_idx;
>> >> > + int n_low, n_up, n;
>> >> > + int base_rate, mul, div, rate;
>> >> >
>> >> > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
>> >> EL
[Intel-gfx] [PATCH] drm/core: Set mode to NULL when connectors in a set drops to 0.
Without this when a MST connector is removed drm_atomic_helper_set_config
can complain about set->mode && !set->num_connectors.
[ cut here ]
WARNING: CPU: 2 PID: 2403 at drivers/gpu/drm/drm_atomic_helper.c:1673
drm_atomic_helper_set_config+0x22e/0x420()
CPU: 2 PID: 2403 Comm: kms_flip Not tainted 4.2.0-rc5 #4233
Hardware name: NUC5i7RYB, BIOS RYBDWi35.86A.0246.2015.0309.1355 03/09/2015
81ac75e8 88004e4ffbf8 81714c34 8000
88004e4ffc38 8107bf81 88004e4ffc48
8800d8ca0690 8800d8d7a080 8800d8cc2290 8800d07bc9f0
Call Trace:
[] dump_stack+0x4f/0x7b
[] warn_slowpath_common+0x81/0xc0
[] warn_slowpath_null+0x15/0x20
[] drm_atomic_helper_set_config+0x22e/0x420
[] ? drm_atomic_helper_plane_set_property+0x84/0xc0
[] drm_mode_set_config_internal+0x61/0x100
[] restore_fbdev_mode+0xbd/0xe0
[] drm_fb_helper_restore_fbdev_mode_unlocked+0x24/0x70
[] intel_fbdev_restore_mode+0x21/0x80 [i915]
[] i915_driver_lastclose+0x9/0x10 [i915]
[] drm_lastclose+0x29/0x130
[] drm_release+0x314/0x500
[] __fput+0xe5/0x1f0
[] fput+0x9/0x10
[] task_work_run+0x88/0xb0
[] do_exit+0x37f/0xa90
[] ? selinux_file_ioctl+0x48/0xc0
[] ? security_file_ioctl+0x3e/0x60
[] do_group_exit+0x40/0xa0
[] SyS_exit_group+0xf/0x10
[] entry_SYSCALL_64_fastpath+0x12/0x6a
---[ end trace 0daf358c49351567 ]---
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_fb_helper.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5875059a7625..418d299f3b12 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -168,11 +168,14 @@ static void remove_from_modeset(struct drm_mode_set *set,
}
set->num_connectors--;
- /* because i915 is pissy about this..
+ /*
* TODO maybe need to makes sure we set it back to !=NULL somewhere?
*/
- if (set->num_connectors == 0)
+ if (set->num_connectors == 0) {
set->fb = NULL;
+ drm_mode_destroy(connector->dev, set->mode);
+ set->mode = NULL;
+ }
}
int drm_fb_helper_remove_one_connector(struct drm_fb_helper *fb_helper,
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/dp/mst: Remove port after removing connector.
The port is removed synchronously, but the connector delayed.
This causes a use after free which can cause a kernel BUG with
slug_debug=FPZU. This is fixed by freeing the port after the
connector.
This fixes a regression introduced with
6b8eeca65b18ae77e175cc2b6571731f0ee413bf
"drm/dp/mst: close deadlock in connector destruction."
Cc: sta...@vger.kernel.org
Cc: Dave Airlie
Signed-off-by: Maarten Lankhorst
---
drivers/gpu/drm/drm_dp_mst_topology.c | 19 +--
include/drm/drm_crtc.h| 2 --
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index b0487c9f018c..eb603f1defc2 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -873,9 +873,10 @@ static void drm_dp_destroy_port(struct kref *kref)
from an EDID retrieval */
if (port->connector) {
mutex_lock(&mgr->destroy_connector_lock);
- list_add(&port->connector->destroy_list,
&mgr->destroy_connector_list);
+ list_add(&port->next, &mgr->destroy_connector_list);
mutex_unlock(&mgr->destroy_connector_lock);
schedule_work(&mgr->destroy_connector_work);
+ return;
}
drm_dp_port_teardown_pdt(port, port->pdt);
@@ -2659,7 +2660,7 @@ static void drm_dp_tx_work(struct work_struct *work)
static void drm_dp_destroy_connector_work(struct work_struct *work)
{
struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
drm_dp_mst_topology_mgr, destroy_connector_work);
- struct drm_connector *connector;
+ struct drm_dp_mst_port *port;
/*
* Not a regular list traverse as we have to drop the destroy
@@ -2668,15 +2669,21 @@ static void drm_dp_destroy_connector_work(struct
work_struct *work)
*/
for (;;) {
mutex_lock(&mgr->destroy_connector_lock);
- connector =
list_first_entry_or_null(&mgr->destroy_connector_list, struct drm_connector,
destroy_list);
- if (!connector) {
+ port = list_first_entry_or_null(&mgr->destroy_connector_list,
struct drm_dp_mst_port, next);
+ if (!port) {
mutex_unlock(&mgr->destroy_connector_lock);
break;
}
- list_del(&connector->destroy_list);
+ list_del(&port->next);
mutex_unlock(&mgr->destroy_connector_lock);
- mgr->cbs->destroy_connector(mgr, connector);
+ mgr->cbs->destroy_connector(mgr, port->connector);
+
+ drm_dp_port_teardown_pdt(port, port->pdt);
+
+ if (!port->input && port->vcpi.vcpi > 0)
+ drm_dp_mst_put_payload_id(mgr, port->vcpi.vcpi);
+ kfree(port);
}
}
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 574656965126..373b1bc6de96 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -745,8 +745,6 @@ struct drm_connector {
uint8_t num_h_tile, num_v_tile;
uint8_t tile_h_loc, tile_v_loc;
uint16_t tile_h_size, tile_v_size;
-
- struct list_head destroy_list;
};
/**
--
2.1.0
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
Hi Jani,
> -Original Message-
> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> Sent: Tuesday, August 11, 2015 3:14 PM
> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> Subject: RE: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
> modeset
>
> On Tue, 11 Aug 2015, "Yang, Libin" wrote:
> > Hi Jani,
> >
> > Thanks for careful reviewing for all the patches, please
> > see my comments.
> >
> >> -Original Message-
> >> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
> >> Sent: Monday, August 10, 2015 8:10 PM
> >> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
> >> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
> >> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
> >> modeset
> >>
> >> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
> >> > From: Libin Yang
> >> >
> >> > When modeset occurs and the TMDS frequency is set to some
> >> > speical value, the N/CTS need to be set manually if audio
> >> > is playing.
> >>
> >> When modeset occurs, we disable everything, and then re-enable
> >> everything, and notify the audio driver every step of the way. IIUC
> >> there should be no audio playing across the modeset, and when the
> >> modeset is complete and we've set the valid ELD, the audio driver
> >> should
> >> call the callback from the earlier patches to set the correct audio
> >> rate.
> >>
> >> Why is this patch needed? Please enlighten me.
> >
> > Please image this scenario: when audio is playing, the gfx driver
> > does the modeset. In this situation, after modeset, audio driver
> > will do nothing and continue playing. And audio driver will not call
> > set_ncts.
>
> Why does the audio driver not do anything here? Whenever there's a
> modeset, the graphics driver will disable audio and invalidate the ELD,
> which should cause an interrupt to notify the audio driver about
> this. This is no different from a hot-unplug, in fact I can't see how
> the audio driver could even detect whether there's been a hotplug or
> not
> when there's a codec disable/enable.
Yes, it will trigger an interrupt to audio driver when unplug
and another interrupt for hotplug. But from audio driver,
we will not stop the playing and resume the playing based
on the hotplug or modeset. The audio is always playing during
the hotplug/modeset.
>
> BR,
> Jani.
>
>
> >
> >>
> >> Also some comments in-line, provided you've convinced me first. ;)
> >>
> >> > Signed-off-by: Libin Yang
> >> > ---
> >> > drivers/gpu/drm/i915/i915_reg.h| 6 ++
> >> > drivers/gpu/drm/i915/intel_audio.c | 42
> >> ++
> >> > 2 files changed, 48 insertions(+)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
> >> b/drivers/gpu/drm/i915/i915_reg.h
> >> > index da2d128..85f3beb 100644
> >> > --- a/drivers/gpu/drm/i915/i915_reg.h
> >> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> >> > @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
> >> > _HSW_AUD_DIG_CNVT_2)
> >> > #define DIP_PORT_SEL_MASK 0x3
> >> >
> >> > +#define _HSW_AUD_STR_DESC_1 0x65084
> >> > +#define _HSW_AUD_STR_DESC_2 0x65184
> >> > +#define AUD_STR_DESC(pipe) _PIPE(pipe, \
> >> > + _HSW_AUD_STR_DESC_1,
> >>\
> >> > + _HSW_AUD_STR_DESC_2)
> >> > +
> >> > #define _HSW_AUD_EDID_DATA_A0x65050
> >> > #define _HSW_AUD_EDID_DATA_B0x65150
> >> > #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
> >> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
> >> b/drivers/gpu/drm/i915/intel_audio.c
> >> > index eddf37f..082f96d 100644
> >> > --- a/drivers/gpu/drm/i915/intel_audio.c
> >> > +++ b/drivers/gpu/drm/i915/intel_audio.c
> >> > @@ -235,6 +235,9 @@ static void
> hsw_audio_codec_enable(struct
> >> drm_connector *connector,
> >> > const uint8_t *eld = connector->eld;
> >> > uint32_t tmp;
> >> > int len, i;
> >> > +int cvt_idx;
> >> > +int n_low, n_up, n;
> >> > +int base_rate, mul, div, rate;
> >> >
> >> > DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
> >> ELD\n",
> >> >pipe_name(pipe), drm_eld_size(eld));
> >> > @@ -267,6 +270,21 @@ static void
> hsw_audio_codec_enable(struct
> >> drm_connector *connector,
> >> > tmp |= AUDIO_ELD_VALID(pipe);
> >> > I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
> >> >
> >> > +if ((mode->clock == 297000) ||
> >> > +(mode->clock == DIV_ROUND_UP(297000 * 1000,
> >> 1001))) {
> >> > +tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
> >> > +cvt_idx = (tmp >> pipe*8) & 0xff;
> >> > +tmp = I915_READ(AUD_STR_DESC(cvt_idx));
> >> > +base_rate = tmp & (1 << 14);
> >> > +if (base_r
Re: [Intel-gfx] [PATCH 8/6] drm/i915/skl: Enable DDI-E
Reviewed-by: Xiong Zhang
thanks
> -Original Message-
> From: Vivi, Rodrigo
> Sent: Saturday, August 8, 2015 8:35 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Vivi, Rodrigo; Zhang, Xiong Y
> Subject: [PATCH 8/6] drm/i915/skl: Enable DDI-E
>
> There are OEMs using DDI-E out there,
> so let's enable it.
>
> Unfortunately there is no detection bit for DDI-E So we need to rely on VBT
> for
> that.
>
> I also need to give credits to Xiong since before seing his approach to check
> info->support_* I was creating an ugly
> vbt->ddie_sfuse_strap in order to propagate the ddi presence info
>
> v2: Rebased as last patch in the series. since all other patches in this
> series are
> needed for anything working propperly on DDI-E.
>
> Credits-to: "Zhang, Xiong Y"
> Cc: "Zhang, Xiong Y"
> Signed-off-by: Rodrigo Vivi
> ---
> drivers/gpu/drm/i915/intel_bios.c| 14 +++---
> drivers/gpu/drm/i915/intel_bios.h| 2 ++
> drivers/gpu/drm/i915/intel_display.c | 9 +
> 3 files changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c
> b/drivers/gpu/drm/i915/intel_bios.c
> index c417973..c5b5a59b 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -898,19 +898,19 @@ static void parse_ddi_port(struct drm_i915_private
> *dev_priv, enum port port,
> /* Each DDI port can have more than one value on the "DVO Port" field,
>* so look for all the possible values for each port and abort if more
>* than one is found. */
> - int dvo_ports[][2] = {
> - {DVO_PORT_HDMIA, DVO_PORT_DPA},
> - {DVO_PORT_HDMIB, DVO_PORT_DPB},
> - {DVO_PORT_HDMIC, DVO_PORT_DPC},
> - {DVO_PORT_HDMID, DVO_PORT_DPD},
> - {DVO_PORT_CRT, -1 /* Port E can only be DVO_PORT_CRT */ },
> + int dvo_ports[][3] = {
> + {DVO_PORT_HDMIA, DVO_PORT_DPA, -1},
> + {DVO_PORT_HDMIB, DVO_PORT_DPB, -1},
> + {DVO_PORT_HDMIC, DVO_PORT_DPC, -1},
> + {DVO_PORT_HDMID, DVO_PORT_DPD, -1},
> + {DVO_PORT_CRT, DVO_PORT_HDMIE, DVO_PORT_DPE},
> };
>
> /* Find the child device to use, abort if more than one found. */
> for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> it = dev_priv->vbt.child_dev + i;
>
> - for (j = 0; j < 2; j++) {
> + for (j = 0; j < 3; j++) {
> if (dvo_ports[port][j] == -1)
> break;
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.h
> b/drivers/gpu/drm/i915/intel_bios.h
> index 77bf1dd..a2ef0df 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -759,6 +759,8 @@ int intel_parse_bios(struct drm_device *dev);
> #define DVO_PORT_DPC 8
> #define DVO_PORT_DPD 9
> #define DVO_PORT_DPA 10
> +#define DVO_PORT_DPE 11
> +#define DVO_PORT_HDMIE 12
> #define DVO_PORT_MIPIA 21
> #define DVO_PORT_MIPIB 22
> #define DVO_PORT_MIPIC 23
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index fcf1230..7bf6209 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13961,6 +13961,15 @@ static void intel_setup_outputs(struct
> drm_device *dev)
> intel_ddi_init(dev, PORT_C);
> if (found & SFUSE_STRAP_DDID_DETECTED)
> intel_ddi_init(dev, PORT_D);
> + /*
> + * On SKL we don't have a way to detect DDI-E so we rely on VBT.
> + */
> + if (IS_SKYLAKE(dev) &&
> + (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp ||
> + dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi ||
> + dev_priv->vbt.ddi_port_info[PORT_E].supports_hdmi))
> + intel_ddi_init(dev, PORT_E);
> +
> } else if (HAS_PCH_SPLIT(dev)) {
> int found;
> dpd_is_edp = intel_dp_is_edp(dev, PORT_D);
> --
> 2.4.3
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in modeset
On Tue, 11 Aug 2015, "Yang, Libin" wrote:
> Hi Jani,
>
> Thanks for careful reviewing for all the patches, please
> see my comments.
>
>> -Original Message-
>> From: Jani Nikula [mailto:jani.nik...@linux.intel.com]
>> Sent: Monday, August 10, 2015 8:10 PM
>> To: Yang, Libin; alsa-de...@alsa-project.org; ti...@suse.de; intel-
>> g...@lists.freedesktop.org; daniel.vet...@ffwll.ch
>> Subject: Re: [Intel-gfx] [PATCH v4 4/4] drm/i915: set proper N/CTS in
>> modeset
>>
>> On Mon, 10 Aug 2015, libin.y...@intel.com wrote:
>> > From: Libin Yang
>> >
>> > When modeset occurs and the TMDS frequency is set to some
>> > speical value, the N/CTS need to be set manually if audio
>> > is playing.
>>
>> When modeset occurs, we disable everything, and then re-enable
>> everything, and notify the audio driver every step of the way. IIUC
>> there should be no audio playing across the modeset, and when the
>> modeset is complete and we've set the valid ELD, the audio driver
>> should
>> call the callback from the earlier patches to set the correct audio
>> rate.
>>
>> Why is this patch needed? Please enlighten me.
>
> Please image this scenario: when audio is playing, the gfx driver
> does the modeset. In this situation, after modeset, audio driver
> will do nothing and continue playing. And audio driver will not call
> set_ncts.
Why does the audio driver not do anything here? Whenever there's a
modeset, the graphics driver will disable audio and invalidate the ELD,
which should cause an interrupt to notify the audio driver about
this. This is no different from a hot-unplug, in fact I can't see how
the audio driver could even detect whether there's been a hotplug or not
when there's a codec disable/enable.
BR,
Jani.
>
>>
>> Also some comments in-line, provided you've convinced me first. ;)
>>
>> > Signed-off-by: Libin Yang
>> > ---
>> > drivers/gpu/drm/i915/i915_reg.h| 6 ++
>> > drivers/gpu/drm/i915/intel_audio.c | 42
>> ++
>> > 2 files changed, 48 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> > index da2d128..85f3beb 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -7030,6 +7030,12 @@ enum skl_disp_power_wells {
>> >_HSW_AUD_DIG_CNVT_2)
>> > #define DIP_PORT_SEL_MASK 0x3
>> >
>> > +#define _HSW_AUD_STR_DESC_1 0x65084
>> > +#define _HSW_AUD_STR_DESC_2 0x65184
>> > +#define AUD_STR_DESC(pipe)_PIPE(pipe, \
>> > + _HSW_AUD_STR_DESC_1,
>> \
>> > + _HSW_AUD_STR_DESC_2)
>> > +
>> > #define _HSW_AUD_EDID_DATA_A 0x65050
>> > #define _HSW_AUD_EDID_DATA_B 0x65150
>> > #define HSW_AUD_EDID_DATA(pipe) _PIPE(pipe, \
>> > diff --git a/drivers/gpu/drm/i915/intel_audio.c
>> b/drivers/gpu/drm/i915/intel_audio.c
>> > index eddf37f..082f96d 100644
>> > --- a/drivers/gpu/drm/i915/intel_audio.c
>> > +++ b/drivers/gpu/drm/i915/intel_audio.c
>> > @@ -235,6 +235,9 @@ static void hsw_audio_codec_enable(struct
>> drm_connector *connector,
>> >const uint8_t *eld = connector->eld;
>> >uint32_t tmp;
>> >int len, i;
>> > + int cvt_idx;
>> > + int n_low, n_up, n;
>> > + int base_rate, mul, div, rate;
>> >
>> >DRM_DEBUG_KMS("Enable audio codec on pipe %c, %u bytes
>> ELD\n",
>> > pipe_name(pipe), drm_eld_size(eld));
>> > @@ -267,6 +270,21 @@ static void hsw_audio_codec_enable(struct
>> drm_connector *connector,
>> >tmp |= AUDIO_ELD_VALID(pipe);
>> >I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, tmp);
>> >
>> > + if ((mode->clock == 297000) ||
>> > + (mode->clock == DIV_ROUND_UP(297000 * 1000,
>> 1001))) {
>> > + tmp = I915_READ(HSW_AUD_PIPE_CONN_SEL_CTRL);
>> > + cvt_idx = (tmp >> pipe*8) & 0xff;
>> > + tmp = I915_READ(AUD_STR_DESC(cvt_idx));
>> > + base_rate = tmp & (1 << 14);
>> > + if (base_rate == 0)
>> > + rate = 48000;
>> > + else
>> > + rate = 44100;
>> > + mul = (tmp & (0x7 << 11)) + 1;
>> > + div = (tmp & (0x7 << 8)) + 1;
>> > + rate = rate * mul / div;
>> > + }
>>
>> This should be abstracted to a separate function.
>
> Yes. I will do it.
>
>>
>> > +
>> >/* Enable timestamps */
>> >tmp = I915_READ(HSW_AUD_CFG(pipe));
>> >tmp &= ~AUD_CONFIG_N_VALUE_INDEX;
>> > @@ -276,6 +294,30 @@ static void hsw_audio_codec_enable(struct
>> drm_connector *connector,
>> >tmp |= AUD_CONFIG_N_VALUE_INDEX;
>> >else
>> >tmp |= audio_config_hdmi_pixel_clock(mode);
>> > +
>> > + if ((mode->clock != 297000) &&
>> > + (mode->clock != DIV_ROUND_UP(297000 * 1000,
>> 1001))) {
>> > + tmp &= ~AUD_CONFIG_N_PROG_ENABLE;
>> > + } else {
>> > + for (i = 0; i < ARRAY_SIZE(aud_ncts); i++) {
>> > +
Re: [Intel-gfx] [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes.
> -Original Message- > From: Vivi, Rodrigo > Sent: Saturday, August 8, 2015 8:34 AM > To: intel-gfx@lists.freedesktop.org > Cc: Vivi, Rodrigo; Zhang, Xiong Y > Subject: [PATCH 7/6] drm/i915/skl: DDI-E and DDI-A shares 4 lanes. > > DDI-A and DDI-E shares the 4 lanes. So when DDI-E is present we need to > configure lane count propperly for both. > > This was based on Sonika's > [PATCH] drm/i915/skl: Select DDIA lane capability based upon vbt > > Credits-to: Sonika Jindal > Cc: Xiong Zhang > Signed-off-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/intel_ddi.c | 12 ++-- > drivers/gpu/drm/i915/intel_dp.c | 8 +--- > 2 files changed, 15 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index 110d546..557cecf 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -3178,7 +3178,15 @@ void intel_ddi_init(struct drm_device *dev, enum > port port) > struct intel_digital_port *intel_dig_port; > struct intel_encoder *intel_encoder; > struct drm_encoder *encoder; > - bool init_hdmi, init_dp; > + bool init_hdmi, init_dp, ddi_e_present; > + > + /* > + * On SKL we don't have a way to detect DDI-E so we rely on VBT. > + */ > + ddie_present = IS_SKYLAKE(dev) && > + (dev_priv->vbt.ddi_port_info[PORT_E].supports_dp || > + dev_priv->vbt.ddi_port_info[PORT_E].supports_dvi || > + dev_priv->vbt.ddi_port_info[PORT_E].supports_hdmi); [Zhang, Xiong Y] ddie_present should be ddi_e_present > > init_hdmi = (dev_priv->vbt.ddi_port_info[port].supports_dvi || >dev_priv->vbt.ddi_port_info[port].supports_hdmi); > @@ -3210,7 +3218,7 @@ void intel_ddi_init(struct drm_device *dev, enum > port port) > intel_dig_port->port = port; > intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) & > (DDI_BUF_PORT_REVERSAL | > -DDI_A_4_LANES); > +ddi_e_present ? 0 : DDI_A_4_LANES); [Zhang, Xiong Y] Sonika's patch will set DDI-A to 4 lanes when DDI-E doesn't exist, I think your patch will do nothing. > > intel_encoder->type = INTEL_OUTPUT_UNKNOWN; > intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2); diff --git > a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index > 3ff2080..7ada79e 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -159,9 +159,11 @@ static u8 intel_dp_max_lane_count(struct intel_dp > *intel_dp) > u8 source_max, sink_max; > > source_max = 4; > - if (HAS_DDI(dev) && intel_dig_port->port == PORT_A && > - (intel_dig_port->saved_port_bits & DDI_A_4_LANES) == 0) > - source_max = 2; > + if (HAS_DDI(dev) && (intel_dig_port->port == PORT_E || > + (intel_dig_port->port == PORT_A && > + (intel_dig_port->saved_port_bits & > +DDI_A_4_LANES) == 0)) > + source_max = 2; [Zhang, Xiong Y] it miss ')' at the end line. > > sink_max = drm_dp_max_lane_count(intel_dp->dpcd); > > -- > 2.4.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
