Re: [Intel-gfx] [PATCH 2/6] drm/i915: add helper functions to get device ptr
On Tue, 14 Jan 2020, "Bharadiya,Pankaj"
wrote:
> Hi Jani,
>
> On Mon, Jan 13, 2020 at 02:14:51PM +0200, Jani Nikula wrote:
>> On Mon, 13 Jan 2020, Pankaj Bharadiya
>> wrote:
>> > We will need struct drm_device pointer to pass it to drm_WARN* calls.
>> >
>> > Add helper functions to exract drm_device pointer from various structs.
>>
>> Please don't do this.
>>
>> We use the helpers we currently have when they involve something more
>> complex than dereferences, such as container_of() or having to use
>> dev_get_drvdata() or pci_get_drvdata().
>
> Removing all the helpers will lead to 3 level access depth wherever
> struct drm_device pointer is needed and unnecessary complicate the
> drm_WARN* calls IMHO.
>
> e.g.: drm_WARN(&vgpu->gvt->dev_priv.drm, 1)
>
> If you are OK with this, I will remove all the helpers and extract
> drm_device pointer while calling drm_WARN*() instead and submit
> updated patch series.
At times you may need to add local variables for struct drm_i915_private
*i915. But I'd rather have that than the helpers.
BR,
Jani.
>
> Thanks,
> Pankaj
>>
>> I really don't want people to use e.g.
>>
>> i915_to_dev(i915)
>>
>> over
>>
>> &i915->drm
>>
>>
>> BR,
>> Jani.
>>
>> >
>> > Signed-off-by: Pankaj Bharadiya
>> > ---
>> > drivers/gpu/drm/i915/display/intel_display_types.h | 14 ++
>> > drivers/gpu/drm/i915/gvt/gvt.h | 5 +
>> > drivers/gpu/drm/i915/i915_drv.h| 11 +++
>> > 3 files changed, 30 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > index 7964b3dc0046..765878718fe9 100644
>> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
>> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
>> > @@ -1521,6 +1521,20 @@ dp_to_i915(struct intel_dp *intel_dp)
>> >return to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
>> > }
>> >
>> > +static inline struct drm_device *enc_to_dev(const struct intel_encoder
>> > *encoder)
>> > +{
>> > + return encoder->base.dev;
>> > +}
>> > +
>> > +static inline struct drm_device *
>> > +crtc_state_to_dev(const struct intel_crtc_state *state)
>> > +{
>> > + struct intel_crtc *crtc = to_intel_crtc(state->uapi.crtc);
>> > + struct drm_i915_private *i915 = to_i915(crtc->base.dev);
>> > +
>> > + return i915_to_dev(i915);
>> > +}
>> > +
>> > static inline struct intel_digital_port *
>> > hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
>> > {
>> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h
>> > b/drivers/gpu/drm/i915/gvt/gvt.h
>> > index b47c6acaf9c0..d257399b075a 100644
>> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
>> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
>> > @@ -348,6 +348,11 @@ static inline struct intel_gvt *to_gvt(struct
>> > drm_i915_private *i915)
>> >return i915->gvt;
>> > }
>> >
>> > +static inline struct drm_device *vgpu_to_dev(const struct intel_vgpu
>> > *vgpu)
>> > +{
>> > + return i915_to_dev(vgpu->gvt->dev_priv);
>> > +}
>> > +
>> > enum {
>> >INTEL_GVT_REQUEST_EMULATE_VBLANK = 0,
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h
>> > index e7be4c3e43c6..bdc89d021ff8 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1325,6 +1325,17 @@ static inline struct drm_i915_private
>> > *pdev_to_i915(struct pci_dev *pdev)
>> >return pci_get_drvdata(pdev);
>> > }
>> >
>> > +static inline struct drm_device *i915_to_dev(struct drm_i915_private
>> > *i915)
>> > +{
>> > + return &i915->drm;
>> > +}
>> > +
>> > +static inline struct drm_device *
>> > +perf_stream_to_dev(const struct i915_perf_stream *stream)
>> > +{
>> > + return i915_to_dev(stream->perf->i915);
>> > +}
>> > +
>> > /* Simple iterator over all initialised engines */
>> > #define for_each_engine(engine__, dev_priv__, id__) \
>> >for ((id__) = 0; \
>>
>> --
>> Jani Nikula, Intel Open Source Graphics Center
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/23] drm/amdgpu: Convert to struct drm_crtc_helper_funcs.get_scanout_position()
Hi
Am 13.01.20 um 19:52 schrieb Alex Deucher:
> On Fri, Jan 10, 2020 at 4:21 AM Thomas Zimmermann wrote:
>>
>> The callback struct drm_driver.get_scanout_position() is deprecated in
>> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert
>> amdgpu over.
>>
>
> I would prefer to just change the signature of
> amdgpu_display_get_crtc_scanoutpos() to match the new API rather than
> wrapping it again.
Will be fixed in v2. I wrapped these functions in amdgpu and radeon to
avoid changes to other, unreleted callers.
Best regards
Thomas
>
> Alex
>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12
>> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 +
>> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 1 +
>> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 1 +
>> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 +
>> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 1 +
>> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 1 +
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
>> 9 files changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> index 4e699071d144..a1e769d4417d 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
>> @@ -914,3 +914,15 @@ int amdgpu_display_crtc_idx_to_irq_type(struct
>> amdgpu_device *adev, int crtc)
>> return AMDGPU_CRTC_IRQ_NONE;
>> }
>> }
>> +
>> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
>> + bool in_vblank_irq, int *vpos,
>> + int *hpos, ktime_t *stime, ktime_t *etime,
>> + const struct drm_display_mode *mode)
>> +{
>> + struct drm_device *dev = crtc->dev;
>> + unsigned int pipe = crtc->index;
>> +
>> + return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
>> + stime, etime, mode);
>> +}
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> index 3f6f14ce1511..0749285dd1c7 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>> @@ -1367,16 +1367,6 @@ int amdgpu_file_to_fpriv(struct file *filp, struct
>> amdgpu_fpriv **fpriv)
>> return 0;
>> }
>>
>> -static bool
>> -amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
>> -bool in_vblank_irq, int *vpos, int *hpos,
>> -ktime_t *stime, ktime_t *etime,
>> -const struct drm_display_mode *mode)
>> -{
>> - return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
>> - stime, etime, mode);
>> -}
>> -
>> static struct drm_driver kms_driver = {
>> .driver_features =
>> DRIVER_USE_AGP | DRIVER_ATOMIC |
>> @@ -1391,7 +1381,6 @@ static struct drm_driver kms_driver = {
>> .enable_vblank = amdgpu_enable_vblank_kms,
>> .disable_vblank = amdgpu_disable_vblank_kms,
>> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
>> - .get_scanout_position = amdgpu_get_crtc_scanout_position,
>> .irq_handler = amdgpu_irq_handler,
>> .ioctls = amdgpu_ioctls_kms,
>> .gem_free_object_unlocked = amdgpu_gem_object_free,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> index eb9975f4decb..37ba07e2feb5 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
>> @@ -612,6 +612,11 @@ void amdgpu_panel_mode_fixup(struct drm_encoder
>> *encoder,
>> struct drm_display_mode *adjusted_mode);
>> int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int
>> crtc);
>>
>> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
>> + bool in_vblank_irq, int *vpos,
>> + int *hpos, ktime_t *stime, ktime_t *etime,
>> + const struct drm_display_mode *mode);
>> +
>> /* fbdev layer */
>> int amdgpu_fbdev_init(struct amdgpu_device *adev);
>> void amdgpu_fbdev_fini(struct amdgpu_device *adev);
>> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> index 40d2ac723dd6..bdc1e0f036d4 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
>> @@ -2685,6 +2685,7 @@ static const struct drm_crtc_helper_funcs
>> dce_v10_0_crtc_helper_funcs = {
>> .prepare = dce_v10_0_crtc_prepare,
>> .commit = dce_v10_0_crtc_commit,
>> .disable =
Re: [PATCH v2 4/7] drm/panfrost: Add support for a second regulator for the GPU
On Fri, Jan 10, 2020 at 7:30 PM Steven Price wrote: > > On 09/01/2020 19:49, Mark Brown wrote: > > On Thu, Jan 09, 2020 at 04:53:02PM +, Steven Price wrote: > >> On 09/01/2020 16:28, Mark Brown wrote: > >>> On Thu, Jan 09, 2020 at 02:14:42PM +, Steven Price wrote: > > > I'm not sure if it's better, but could we just encode the list of > regulators into device tree. I'm a bit worried about special casing an > "sram" regulator given that other platforms might have a similar > situation but call the second regulator a different name. > > > >>> Obviously the list of regulators bound on a given platform is encoded in > >>> the device tree but you shouldn't really be relying on that to figure > >>> out what to request in the driver - the driver should know what it's > >>> expecting. > > > >> From a driver perspective we don't expect to have to worry about power > >> domains/multiple regulators - the hardware provides a bunch of power > >> registers to turn on/off various parts of the hardware and this should be > >> linked (in hardware) to a PDC which sorts it out. The GPU/PDC handles the > >> required sequencing. So it *should* be a case of turn power/clocks on and > >> go. > > > > Ah, the well abstracted and consistent hardware with which we are all so > > fortunate to work :) . More seriously perhaps the thing to do here is > > create a driver that provides a soft PDC and then push all the special > > case handling into that? It can then get instantiated based on the > > compatible or perhaps represented directly in the device tree if that > > makes sense. > > That makes sense to me. > > >> However certain integrations may have quirks such that there are physically > >> multiple supplies. These are expected to all be turned on before using the > >> GPU. Quite how this is best represented is something I'm not sure about. > > > > If they're always on and don't ever change then that's really easy to > > represent in the DT without involving drivers, it's when you need to > > actively manage them that it's more effort. > > Sorry, I should have been more clear. They are managed as a group - so > either the entire GPU is powered off, or powered on. There's no support > in Panfrost or mali_kbase for attempting to power part of the GPU. > > >>> Bear in mind that getting regulator stuff wrong can result > >>> in physical damage to the system so it pays to be careful and to > >>> consider that platform integrators have a tendency to rely on things > >>> that just happen to work but aren't a good idea or accurate > >>> representations of the system. It's certainly *possible* to do > >>> something like that, the information is there, but I would not in any > >>> way recommend doing things that way as it's likely to not be robust. > > > >>> The possibility that the regulator setup may vary on other platforms > >>> (which I'd expect TBH) does suggest that just requesting a bunch of > >>> supply names optionally and hoping that we got all the ones that are > >>> important on a given platform is going to lead to trouble down the line. > > > >> Certainly if we miss a regulator the GPU isn't going to work properly (some > >> cores won't be able to power up successfully). However at the moment the > >> driver will happily do this if someone provides it with a DT which includes > >> regulators that it doesn't know about. So I'm not sure how adding special > >> case code for a SoC would help here. > > > > I thought this SoC neeed to vary the voltage on both rails as part of > > the power management? Things like that can lead to hardware damage if > > we go out of spec far enough for long enough - there can be requirements > > like keeping one rail a certain voltage above another or whatever. > > Yes, you are correct. My concern is that a DT which specifies a new > regulator (e.g. "sram2") would be accepted by an old kernel (because it > wouldn't know to look for the new regulator) but wouldn't know to > control the regulator. It could then create a situation which puts the > board out of spec - potentially in a damaging way. Hence I'd like to > express the regulator structure in such a way that old kernels wouldn't > "half-work". Your "soft-PDC" approach would seem to fit that requirement. FYI, I sent a v3 here: https://patchwork.kernel.org/patch/11331373/ that addresses _some_ of these concerns. I'm not quite sure how to describe the regulators in a way that we can check that the device tree does not specific extra ones (apart from doing some string matching on all properties?), and I don't think I'm best placed to implement the soft-PDC idea. See my comment on that patch. Thanks! ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 7/7, RFC] drm/panfrost: devfreq: Add support for 2 regulators
The Bifrost GPU on MT8183 uses 2 regulators (core and SRAM) for
devfreq, and provides OPP table with 2 sets of voltages.
TODO: This is incomplete as we'll need add support for setting
a pair of voltages as well.
Signed-off-by: Nicolas Boichat
---
drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +
drivers/gpu/drm/panfrost/panfrost_device.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 413987038fbfccb..9c0987a3d71c597 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -79,6 +79,21 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
+ /* If we have 2 regulator, we need an OPP table with 2 voltages. */
+ if (pfdev->comp->num_supplies > 1) {
+ pfdev->devfreq.dev_opp_table =
+ dev_pm_opp_set_regulators(dev,
+ pfdev->comp->supply_names,
+ pfdev->comp->num_supplies);
+ if (IS_ERR(pfdev->devfreq.dev_opp_table)) {
+ ret = PTR_ERR(pfdev->devfreq.dev_opp_table);
+ pfdev->devfreq.dev_opp_table = NULL;
+ dev_err(dev,
+ "Failed to init devfreq opp table: %d\n", ret);
+ return ret;
+ }
+ }
+
ret = dev_pm_opp_of_add_table(dev);
if (ret == -ENODEV) /* Optional, continue without devfreq */
return 0;
@@ -119,6 +134,8 @@ void panfrost_devfreq_fini(struct panfrost_device *pfdev)
if (pfdev->devfreq.cooling)
devfreq_cooling_unregister(pfdev->devfreq.cooling);
dev_pm_opp_of_remove_table(&pfdev->pdev->dev);
+ if (pfdev->devfreq.dev_opp_table)
+ dev_pm_opp_put_regulators(pfdev->devfreq.dev_opp_table);
}
void panfrost_devfreq_resume(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 143eab57180a2e1..30ba11cbf600847 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -108,6 +108,7 @@ struct panfrost_device {
struct {
struct devfreq *devfreq;
struct thermal_cooling_device *cooling;
+ struct opp_table *dev_opp_table;
ktime_t busy_time;
ktime_t idle_time;
ktime_t time_last_update;
--
2.25.0.rc1.283.g88dfdc4193-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 4/7] drm/panfrost: Add support for multiple regulators
Some GPUs, namely, the bifrost/g72 part on MT8183, have a second
regulator for their SRAM, let's add support for that.
We extend the framework in a generic manner so that we could
support more than 2 regulators, if required.
Signed-off-by: Nicolas Boichat
---
v3:
- Make this more generic, by allowing any number of regulators
(in practice we fix the maximum number of regulators to 2, but
this could be increased easily).
- We only probe the second regulator if the device tree matching
data asks for it.
- I couldn't find a way to detect the number of regulators in the
device tree, if we wanted to refuse to probe the device if there
are too many regulators, which might be required for safety, see
the thread on v2 [1].
- The discussion also included the idea of a separate device tree
entry for a "soft PDC", or at least a separate driver. I'm not
sure to understand the full picture, and how different vendors
implement this, so I'm still integrating everything in the main
driver. I'd be happy to try to make mt8183 fit into such a
framework after it's created, but I don't think I'm best placed
to implement (and again, the main purpose of this was to test
if the binding is correct).
[1] https://patchwork.kernel.org/patch/11322839/
drivers/gpu/drm/panfrost/panfrost_device.c | 25 ---
drivers/gpu/drm/panfrost/panfrost_device.h | 15 +++-
drivers/gpu/drm/panfrost/panfrost_drv.c| 28 +++---
3 files changed, 50 insertions(+), 18 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index 238fb6d54df4732..c30e0a3772a4f57 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -87,18 +87,26 @@ static void panfrost_clk_fini(struct panfrost_device *pfdev)
static int panfrost_regulator_init(struct panfrost_device *pfdev)
{
- int ret;
+ int ret, i;
- pfdev->regulator = devm_regulator_get(pfdev->dev, "mali");
- if (IS_ERR(pfdev->regulator)) {
- ret = PTR_ERR(pfdev->regulator);
- dev_err(pfdev->dev, "failed to get regulator: %d\n", ret);
+ BUG_ON(pfdev->comp->num_supplies > ARRAY_SIZE(pfdev->regulators));
+
+ for (i = 0; i < pfdev->comp->num_supplies; i++) {
+ pfdev->regulators[i].supply = pfdev->comp->supply_names[i];
+ }
+
+ ret = devm_regulator_bulk_get(pfdev->dev,
+ pfdev->comp->num_supplies,
+ pfdev->regulators);
+ if (ret < 0) {
+ dev_err(pfdev->dev, "failed to get regulators: %d\n", ret);
return ret;
}
- ret = regulator_enable(pfdev->regulator);
+ ret = regulator_bulk_enable(pfdev->comp->num_supplies,
+ pfdev->regulators);
if (ret < 0) {
- dev_err(pfdev->dev, "failed to enable regulator: %d\n", ret);
+ dev_err(pfdev->dev, "failed to enable regulators: %d\n", ret);
return ret;
}
@@ -107,7 +115,8 @@ static int panfrost_regulator_init(struct panfrost_device
*pfdev)
static void panfrost_regulator_fini(struct panfrost_device *pfdev)
{
- regulator_disable(pfdev->regulator);
+ regulator_bulk_disable(pfdev->comp->num_supplies,
+ pfdev->regulators);
}
int panfrost_device_init(struct panfrost_device *pfdev)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 06713811b92cdf7..021f063ffb3747f 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -7,6 +7,7 @@
#include
#include
+#include
#include
#include
#include
@@ -19,6 +20,7 @@ struct panfrost_job;
struct panfrost_perfcnt;
#define NUM_JOB_SLOTS 3
+#define MAX_REGULATORS 2
struct panfrost_features {
u16 id;
@@ -51,6 +53,16 @@ struct panfrost_features {
unsigned long hw_issues[64 / BITS_PER_LONG];
};
+/*
+ * Features that cannot be automatically detected and need matching using the
+ * compatible string, typically SoC-specific.
+ */
+struct panfrost_compatible {
+ /* Supplies count and names. */
+ int num_supplies;
+ const char * const *supply_names;
+};
+
struct panfrost_device {
struct device *dev;
struct drm_device *ddev;
@@ -59,10 +71,11 @@ struct panfrost_device {
void __iomem *iomem;
struct clk *clock;
struct clk *bus_clock;
- struct regulator *regulator;
+ struct regulator_bulk_data regulators[MAX_REGULATORS];
struct reset_control *rstc;
struct panfrost_features features;
+ const struct panfrost_compatible* comp;
spinlock_t as_lock;
unsigned long as_in_use_mask;
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
[PATCH v3 6/7, RFC] drm/panfrost: Add mt8183-mali compatible string
For testing only, the driver doesn't really work yet, AFAICT.
Signed-off-by: Nicolas Boichat
---
v3:
- Match mt8183-mali instead of bifrost, as we require special
handling for the 2 regulators and 3 power domains.
drivers/gpu/drm/panfrost/panfrost_drv.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 42b87e29e605149..3379a3ea754ccde 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -592,6 +592,13 @@ static const struct panfrost_compatible default_data = {
.num_pm_domains = 1, /* optional */
};
+const char * const mediatek_mt8183_supplies[] = { "mali", "sram" };
+static const struct panfrost_compatible mediatek_mt8183_data = {
+ .num_supplies = ARRAY_SIZE(mediatek_mt8183_supplies),
+ .supply_names = mediatek_mt8183_supplies,
+ .num_pm_domains = 3,
+};
+
static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t604", .data = &default_data, },
{ .compatible = "arm,mali-t624", .data = &default_data, },
@@ -602,6 +609,8 @@ static const struct of_device_id dt_match[] = {
{ .compatible = "arm,mali-t830", .data = &default_data, },
{ .compatible = "arm,mali-t860", .data = &default_data, },
{ .compatible = "arm,mali-t880", .data = &default_data, },
+ { .compatible = "mediatek,mt8183-mali",
+ .data = &mediatek_mt8183_data },
{}
};
MODULE_DEVICE_TABLE(of, dt_match);
--
2.25.0.rc1.283.g88dfdc4193-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 3/7] drm/panfrost: Improve error reporting in panfrost_gpu_power_on
It is useful to know which component cannot be powered on. Signed-off-by: Nicolas Boichat Reviewed-by: Steven Price Reviewed-by: Alyssa Rosenzweig --- Was useful when trying to probe Bifrost GPU, to understand what issue we are facing. v3: - Rebased on https://patchwork.kernel.org/patch/11325689/ drivers/gpu/drm/panfrost/panfrost_gpu.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_gpu.c b/drivers/gpu/drm/panfrost/panfrost_gpu.c index 460fc190de6e815..856f2fd1fa8ed27 100644 --- a/drivers/gpu/drm/panfrost/panfrost_gpu.c +++ b/drivers/gpu/drm/panfrost/panfrost_gpu.c @@ -308,17 +308,20 @@ void panfrost_gpu_power_on(struct panfrost_device *pfdev) gpu_write(pfdev, L2_PWRON_LO, pfdev->features.l2_present); ret = readl_relaxed_poll_timeout(pfdev->iomem + L2_READY_LO, val, val == pfdev->features.l2_present, 100, 1000); + if (ret) + dev_err(pfdev->dev, "error powering up gpu L2"); gpu_write(pfdev, SHADER_PWRON_LO, pfdev->features.shader_present); - ret |= readl_relaxed_poll_timeout(pfdev->iomem + SHADER_READY_LO, + ret = readl_relaxed_poll_timeout(pfdev->iomem + SHADER_READY_LO, val, val == pfdev->features.shader_present, 100, 1000); + if (ret) + dev_err(pfdev->dev, "error powering up gpu shader"); gpu_write(pfdev, TILER_PWRON_LO, pfdev->features.tiler_present); - ret |= readl_relaxed_poll_timeout(pfdev->iomem + TILER_READY_LO, + ret = readl_relaxed_poll_timeout(pfdev->iomem + TILER_READY_LO, val, val == pfdev->features.tiler_present, 100, 1000); - if (ret) - dev_err(pfdev->dev, "error powering up gpu"); + dev_err(pfdev->dev, "error powering up gpu tiler"); } void panfrost_gpu_power_off(struct panfrost_device *pfdev) -- 2.25.0.rc1.283.g88dfdc4193-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 2/7] arm64: dts: mt8183: Add node for the Mali GPU
Add a basic GPU node for mt8183.
Signed-off-by: Nicolas Boichat
Reviewed-by: Alyssa Rosenzweig
---
Upstreaming what matches existing bindings from our Chromium OS tree:
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/arch/arm64/boot/dts/mediatek/mt8183.dtsi#1348
The evb part of this change depends on this patch to add PMIC dtsi:
https://patchwork.kernel.org/patch/10928161/
The binding we use with out-of-tree Mali drivers includes more
clocks, this is used for devfreq: the out-of-tree driver switches
clk_mux to clk_sub_parent (26Mhz), adjusts clk_main_parent, then
switches clk_mux back to clk_main_parent:
(see
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#423)
clocks =
<&topckgen CLK_TOP_MFGPLL_CK>,
<&topckgen CLK_TOP_MUX_MFG>,
<&clk26m>,
<&mfgcfg CLK_MFG_BG3D>;
clock-names =
"clk_main_parent",
"clk_mux",
"clk_sub_parent",
"subsys_mfg_cg";
v3:
- No changes
v2:
- Use sram instead of mali_sram as SRAM supply name.
- Rename mali@ to gpu@.
arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 ++
arch/arm64/boot/dts/mediatek/mt8183.dtsi| 104
2 files changed, 111 insertions(+)
diff --git a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
index 1fb195c683c3d01..7d609e0cd9b4975 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
+++ b/arch/arm64/boot/dts/mediatek/mt8183-evb.dts
@@ -7,6 +7,7 @@
/dts-v1/;
#include "mt8183.dtsi"
+#include "mt6358.dtsi"
/ {
model = "MediaTek MT8183 evaluation board";
@@ -30,6 +31,12 @@ &auxadc {
status = "okay";
};
+&gpu {
+ supply-names = "mali", "sram";
+ mali-supply = <&mt6358_vgpu_reg>;
+ sram-supply = <&mt6358_vsram_gpu_reg>;
+};
+
&i2c0 {
pinctrl-names = "default";
pinctrl-0 = <&i2c_pins_0>;
diff --git a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
index 1ade9153e5c265b..4da3f1ed1c15bf3 100644
--- a/arch/arm64/boot/dts/mediatek/mt8183.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8183.dtsi
@@ -597,6 +597,110 @@ mfgcfg: syscon@1300 {
#clock-cells = <1>;
};
+ gpu: gpu@1304 {
+ compatible = "mediatek,mt8183-mali", "arm,mali-bifrost";
+ reg = <0 0x1304 0 0x4000>;
+ interrupts =
+ ,
+ ,
+ ;
+ interrupt-names = "job", "mmu", "gpu";
+
+ clocks = <&topckgen CLK_TOP_MFGPLL_CK>;
+
+ power-domains =
+ <&scpsys MT8183_POWER_DOMAIN_MFG_CORE0>,
+ <&scpsys MT8183_POWER_DOMAIN_MFG_CORE1>,
+ <&scpsys MT8183_POWER_DOMAIN_MFG_2D>;
+
+ operating-points-v2 = <&gpu_opp_table>;
+ };
+
+ gpu_opp_table: opp_table0 {
+ compatible = "operating-points-v2";
+ opp-shared;
+
+ opp-3 {
+ opp-hz = /bits/ 64 <3>;
+ opp-microvolt = <625000>, <85>;
+ };
+
+ opp-32000 {
+ opp-hz = /bits/ 64 <32000>;
+ opp-microvolt = <631250>, <85>;
+ };
+
+ opp-34000 {
+ opp-hz = /bits/ 64 <34000>;
+ opp-microvolt = <637500>, <85>;
+ };
+
+ opp-36000 {
+ opp-hz = /bits/ 64 <36000>;
+ opp-microvolt = <643750>, <85>;
+ };
+
+ opp-38000 {
+ opp-hz = /bits/ 64 <38000>;
+ opp-microvolt = <65>, <85>;
+ };
+
+ opp-4 {
+ opp-hz = /bits/ 64 <4>;
+ opp-microvolt = <656250>, <85>;
+ };
+
+ opp-42000 {
+ opp-hz = /bits/ 64 <42000>;
+ opp-microvolt = <662500>, <85>;
+ };
+
+ opp-46000 {
+ opp-hz = /bits/ 64 <46000>;
+ opp-microvolt = <675000>, <85>;
+ };
+
+ opp-5 {
+ opp-hz = /bits/ 64 <5>;
+
[PATCH v3 5/7] drm/panfrost: Add support for multiple power domains
When there is a single power domain per device, the core will
ensure the power domain is switched on (so it is technically
equivalent to having not power domain specified at all).
However, when there are multiple domains, as in MT8183 Bifrost
GPU, we need to handle them in driver code.
Signed-off-by: Nicolas Boichat
---
The downstream driver we use on chromeos-4.19 currently uses 2
additional devices in device tree to accomodate for this [1], but
I believe this solution is cleaner.
[1]
https://chromium.googlesource.com/chromiumos/third_party/kernel/+/refs/heads/chromeos-4.19/drivers/gpu/arm/midgard/platform/mediatek/mali_kbase_runtime_pm.c#31
v3:
- Use the compatible matching data to specify the number of power
domains. Note that setting 0 or 1 in num_pm_domains is equivalent
as the core will handle these 2 cases in the exact same way
(automatically, without driver intervention), and there should
be no adverse consequence in this case (the concern is about
switching on only some power domains and not others).
drivers/gpu/drm/panfrost/panfrost_device.c | 95 --
drivers/gpu/drm/panfrost/panfrost_device.h | 9 ++
drivers/gpu/drm/panfrost/panfrost_drv.c| 1 +
3 files changed, 97 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c
b/drivers/gpu/drm/panfrost/panfrost_device.c
index c30e0a3772a4f57..7c9766f76cc7689 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
#include "panfrost_device.h"
@@ -119,6 +120,75 @@ static void panfrost_regulator_fini(struct panfrost_device
*pfdev)
pfdev->regulators);
}
+static void panfrost_pm_domain_fini(struct panfrost_device *pfdev)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pfdev->pm_domain_devs); i++) {
+ if (!pfdev->pm_domain_devs[i])
+ break;
+
+ if (pfdev->pm_domain_links[i])
+ device_link_del(pfdev->pm_domain_links[i]);
+
+ dev_pm_domain_detach(pfdev->pm_domain_devs[i], true);
+ }
+}
+
+static int panfrost_pm_domain_init(struct panfrost_device *pfdev)
+{
+ int err;
+ int i, num_domains;
+
+ num_domains = of_count_phandle_with_args(pfdev->dev->of_node,
+"power-domains",
+"#power-domain-cells");
+
+ /*
+* Single domain is handled by the core, and, if only a single power
+* the power domain is requested, the property is optional.
+*/
+ if (num_domains < 2 && pfdev->comp->num_pm_domains < 2)
+ return 0;
+
+ if (num_domains != pfdev->comp->num_pm_domains) {
+ dev_err(pfdev->dev,
+ "Incorrect number of power domains: %d provided, %d
needed\n",
+ num_domains, pfdev->comp->num_pm_domains);
+ return -EINVAL;
+ }
+
+ BUG_ON(num_domains > ARRAY_SIZE(pfdev->pm_domain_devs));
+
+ for (i = 0; i < num_domains; i++) {
+ pfdev->pm_domain_devs[i] =
+ dev_pm_domain_attach_by_id(pfdev->dev, i);
+ if (IS_ERR(pfdev->pm_domain_devs[i])) {
+ err = PTR_ERR(pfdev->pm_domain_devs[i]);
+ pfdev->pm_domain_devs[i] = NULL;
+ dev_err(pfdev->dev,
+ "failed to get pm-domain %d: %d\n", i, err);
+ goto err;
+ }
+
+ pfdev->pm_domain_links[i] = device_link_add(pfdev->dev,
+ pfdev->pm_domain_devs[i], DL_FLAG_PM_RUNTIME |
+ DL_FLAG_STATELESS | DL_FLAG_RPM_ACTIVE);
+ if (!pfdev->pm_domain_links[i]) {
+ dev_err(pfdev->pm_domain_devs[i],
+ "adding device link failed!\n");
+ err = -ENODEV;
+ goto err;
+ }
+ }
+
+ return 0;
+
+err:
+ panfrost_pm_domain_fini(pfdev);
+ return err;
+}
+
int panfrost_device_init(struct panfrost_device *pfdev)
{
int err;
@@ -149,37 +219,45 @@ int panfrost_device_init(struct panfrost_device *pfdev)
goto err_out1;
}
+ err = panfrost_pm_domain_init(pfdev);
+ if (err) {
+ dev_err(pfdev->dev, "pm_domain init failed %d\n", err);
+ goto err_out2;
+ }
+
res = platform_get_resource(pfdev->pdev, IORESOURCE_MEM, 0);
pfdev->iomem = devm_ioremap_resource(pfdev->dev, res);
if (IS_ERR(pfdev->iomem)) {
dev_err(pfdev->dev, "failed to ioremap iomem\n");
err = PTR_ERR(pfdev->iomem);
- goto err_out2;
+ goto err_out3;
}
err = panfrost
[PATCH v3 1/7] dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183
Define a compatible string for the Mali Bifrost GPU found in Mediatek's MT8183 SoCs. Signed-off-by: Nicolas Boichat Reviewed-by: Alyssa Rosenzweig --- v3: - No change .../bindings/gpu/arm,mali-bifrost.yaml | 18 ++ 1 file changed, 18 insertions(+) diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 4ea6a8789699709..9e095608d2d98f0 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -17,6 +17,7 @@ properties: items: - enum: - amlogic,meson-g12a-mali + - mediatek,mt8183-mali - realtek,rtd1619-mali - rockchip,px30-mali - const: arm,mali-bifrost # Mali Bifrost GPU model/revision is fully discoverable @@ -62,6 +63,23 @@ allOf: minItems: 2 required: - resets + - if: + properties: +compatible: + contains: +const: mediatek,mt8183-mali +then: + properties: +sram-supply: true +power-domains: + description: +List of phandle and PM domain specifier as documented in +Documentation/devicetree/bindings/power/power_domain.txt + minItems: 3 + maxItems: 3 + required: +- sram-supply +- power-domains examples: - | -- 2.25.0.rc1.283.g88dfdc4193-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/7] Add dts for mt8183 GPU (and misc panfrost patches)
Hi! Follow-up on the v2: https://patchwork.kernel.org/cover/11322801/ . The main purpose of this series is to upstream the dts change and the binding document, but I wanted to see how far I could probe the GPU, to check that the binding is indeed correct. The rest of the patches are RFC/work-in-progress, but I think some of them could already be picked up. So this is tested on MT8183 with a chromeos-4.19 kernel, and a ton of backports to get the latest panfrost driver (I should probably try on linux-next at some point but this was the path of least resistance). I tested it as a module as it's more challenging (originally probing would work built-in, on boot, but not as a module, as I didn't have the power domain changes, and all power domains are on by default during boot). Probing logs looks like this, currently. They look sane. [ 501.319728] panfrost 1304.gpu: clock rate = 51170 [ 501.320041] panfrost 1304.gpu: Linked as a consumer to regulator.14 [ 501.320102] panfrost 1304.gpu: Linked as a consumer to regulator.31 [ 501.320651] panfrost 1304.gpu: Linked as a consumer to genpd:0:1304.gpu [ 501.320954] panfrost 1304.gpu: Linked as a consumer to genpd:1:1304.gpu [ 501.321062] panfrost 1304.gpu: Linked as a consumer to genpd:2:1304.gpu [ 501.321734] panfrost 1304.gpu: mali-g72 id 0x6221 major 0x0 minor 0x3 status 0x0 [ 501.321741] panfrost 1304.gpu: features: ,13de77ff, issues: ,0400 [ 501.321747] panfrost 1304.gpu: Features: L2:0x07120206 Shader:0x Tiler:0x0809 Mem:0x1 MMU:0x2830 AS:0xff JS:0x7 [ 501.321752] panfrost 1304.gpu: shader_present=0x7 l2_present=0x1 [ 501.324951] [drm] Initialized panfrost 1.1.0 20180908 for 1304.gpu on minor 2 Some more changes are still required to get devfreq working, and of course I do not have a userspace driver to test this with. Thanks! Nicolas v3 (see individual patches, too): - Match a specific mediatek,mt8183-mali instead of the generic bifrost, as this instance requires 2 special cases: - 2 regulators - 3 power domains v2: - Use sram instead of mali_sram as SRAM supply name. - Rename mali@ to gpu@. - Add dt-bindings changes - Stacking patches after the device tree change that allow basic probing (still incomplete and broken). Nicolas Boichat (7): dt-bindings: gpu: mali-bifrost: Add Mediatek MT8183 arm64: dts: mt8183: Add node for the Mali GPU drm/panfrost: Improve error reporting in panfrost_gpu_power_on drm/panfrost: Add support for multiple regulators drm/panfrost: Add support for multiple power domains RFC: drm/panfrost: Add mt8183-mali compatible string RFC: drm/panfrost: devfreq: Add support for 2 regulators .../bindings/gpu/arm,mali-bifrost.yaml| 18 +++ arch/arm64/boot/dts/mediatek/mt8183-evb.dts | 7 + arch/arm64/boot/dts/mediatek/mt8183.dtsi | 104 +++ drivers/gpu/drm/panfrost/panfrost_devfreq.c | 17 +++ drivers/gpu/drm/panfrost/panfrost_device.c| 120 +++--- drivers/gpu/drm/panfrost/panfrost_device.h| 25 +++- drivers/gpu/drm/panfrost/panfrost_drv.c | 38 -- drivers/gpu/drm/panfrost/panfrost_gpu.c | 11 +- 8 files changed, 310 insertions(+), 30 deletions(-) -- 2.25.0.rc1.283.g88dfdc4193-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206141] VCE UVD ring test failed -110
https://bugzilla.kernel.org/show_bug.cgi?id=206141 --- Comment #4 from Janpieter Sollie (janpieter.sol...@dommel.be) --- Hi Alex, Thank you for the feedback. Tried that one as well. When I already multiplied the timeout by 4, guess it will be the bad state then. Is there any way I could reset the state before the ring tests begin? FYI: I'm tried different firmware revisions already, some of them seem to impact the situation: sometimes UVD fails, sometimes VCE fails, but none of them seem to work for both. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM driver and runtime suspend-resume handling?
On Mon, Jan 13, 2020 at 01:03:11PM +0200, Jyri Sarha wrote: > Hi, > While working with CRTC color related properties (gamma and CTM for > instance) and making them persistent over suspend-resume cycle it > occurred to me if I am just wasting resources by storing the property > values in the driver and restoring them in dev_pm_ops runtime_resume().. > > Wouldn't it work if I would just: > > 1. Add a flag in the driver to indicate that the context may have been > lost since the previous atomic commit and set in runtime_resume(). > > 2. And write the color properties to HW if the context lost flag is set > even if the drm_crtc_state color_mgmt_changed is false. Still feels a bit too complicated, but might be needed for your hw. The usual approach is: - runtime pm within modeset enable/disable. Since atomic helpers always enable the entire pipeline for a crtc enable/disable you can put the runtime pm into each component (drm_crtc/encoder/bridge/...). - just unconditionally restore everything in modeset enable, assuming everything got lost. - drm_atomic_helper_suspend/resume for system suspend/resume. And you should be covevered. State save/restore in your driver code is indeed an anti-pattern for modeset drivers, don't do that - ime at least with fragile hw you'll get divergence between the two paths in minor details, with some really hard to track down bugs as a result. Ime tracking state changes and trying to be clever with when to restore stuff (maybe outside of some atomic flip fastpath) in driver code only attracts bugs :-) > The color property values are there despite the color_mgmt_changed == > false, aren't they? Yes. Cheers, Daniel > > Best regards, > Jyri > > -- > Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. > Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/cirrus: Let DRM core send VBLANK events
On Mon, Jan 13, 2020 at 10:01 AM Thomas Zimmermann wrote:
>
> Hi
>
> Am 13.01.20 um 00:00 schrieb Daniel Vetter:
> > On Fri, Jan 10, 2020 at 12:57:07PM +0100, Thomas Zimmermann wrote:
> >> In drm_atomic_helper_fake_vblank() the DRM core sends out VBLANK
> >> events if struct drm_crtc_state.no_vblank is enabled. Replace cirrus'
> >> VBLANK events with the DRM core's functionality.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> ---
> >> drivers/gpu/drm/cirrus/cirrus.c | 10 ++
> >> 1 file changed, 2 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/cirrus/cirrus.c
> >> b/drivers/gpu/drm/cirrus/cirrus.c
> >> index 248c9f765c45..4a1729aa7e53 100644
> >> --- a/drivers/gpu/drm/cirrus/cirrus.c
> >> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> >> @@ -38,7 +38,6 @@
> >> #include
> >> #include
> >> #include
> >> -#include
> >>
> >> #define DRIVER_NAME "cirrus"
> >> #define DRIVER_DESC "qemu cirrus vga"
> >> @@ -415,6 +414,8 @@ static void cirrus_pipe_enable(struct
> >> drm_simple_display_pipe *pipe,
> >> {
> >> struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
> >>
> >> +crtc_state->no_vblank = true;
> >
> > Huh, nice untended use of this stuff ... We've added this for writeback,
> > but I guess it can be used for anything that's a virtual connector ...
>
> Oh, 'improved by accident'.
>
> I'm not quite sure what you mean by virtual connector, but it should
> work with any CRTC without VBLANK support. At least I've never seen any
> problem with ast and udl. I'll update the docs accordingly.
There's a pretty huge difference between "real vblank support", which
means the various vblank/crtc_sequence ioctls work, and the fake
vblank even stuff we send out for all drivers right away to fulfill
the atomic uapi event requirements. We'll need to highlight that stuff
in the documentation I think ... On second thought, maybe we could
change the atomic helpers to automatically set no_vblank when the
driver/crtc doesn't support real vblanks. But is even more involved
(but I think might also be the even neater soluation, but hard to be
sure without typing it all up). Oh and because hilarious historical
accents "no real vblank" = drm_dev->num_crtcs == 0, which we might
want to put into a helper if we leak this out of the drm_vblank.c
source files.
Cheers, Daniel
>
> Best regards
> Thomas
>
> >
> > I've also spotted that you've done this same trick for ast & udl already.
> > But I think before we roll this out massively we should make this
> > official. Can you pls do a patch to update the kerneldoc for @no_vblank
> > that virtual hw can also use this stuff?
> >
> > Also, computing state values in atomic_commit code is kinda uncool and
> > fraught with peril - design assumption is that with some very few
> > exceptions (which are kinda awkward, would be nice to make state pointers
> > const) all the core and helper codes that state structures stay unchanged
> > after atomic_check completed. This should be computed in atomic_check (like
> > vc4
> > does). Can you pls also include patches to update ast and udl in this
> > series?
> >
> > Thanks, Daniel
> >
> >
> >> +
> >> cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
> >> cirrus_fb_blit_fullscreen(plane_state->fb);
> >> }
> >> @@ -434,13 +435,6 @@ static void cirrus_pipe_update(struct
> >> drm_simple_display_pipe *pipe,
> >>
> >> if (drm_atomic_helper_damage_merged(old_state, state, &rect))
> >> cirrus_fb_blit_rect(pipe->plane.state->fb, &rect);
> >> -
> >> -if (crtc->state->event) {
> >> -spin_lock_irq(&crtc->dev->event_lock);
> >> -drm_crtc_send_vblank_event(crtc, crtc->state->event);
> >> -crtc->state->event = NULL;
> >> -spin_unlock_irq(&crtc->dev->event_lock);
> >> -}
> >> }
> >>
> >> static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = {
> >> --
> >> 2.24.1
> >>
> >
>
> --
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[GIT PULL] mediatek drm next for 5.6
Hi, Dave, Daniel: This fix non-smooth cursor problem, add cmdq support, add ctm property support and some refinement. Regards, CK The following changes since commit e42617b825f8073569da76dc4510bfa019b1c35a: Linux 5.5-rc1 (2019-12-08 14:57:55 -0800) are available in the Git repository at: https://github.com/ckhu-mediatek/linux.git-tags.git tags/mediatek-drm-next-5.6 for you to fetch changes up to d04a07a8dc93dfc40b3a41c977db78ba340f209d: drm/mediatek: Fix indentation in Makefile (2020-01-10 14:10:49 +0800) Mediatek DRM Next for Linux 5.6 Bibby Hsieh (11): drm/mediatek: use DRM core's atomic commit helper drm/mediatek: handle events when enabling/disabling crtc drm/mediatek: update cursors by using async atomic update drm/mediatek: disable all the planes in atomic_disable drm/mediatek: remove unused external function soc: mediatek: cmdq: remove OR opertaion from err return soc: mediatek: cmdq: define the instruction struct soc: mediatek: cmdq: add polling function soc: mediatek: cmdq: add cmdq_dev_get_client_reg function drm/mediatek: support CMDQ interface in ddp component drm/mediatek: apply CMDQ control flow CK Hu (1): Merge tag 'v5.5-next-cmdq-stable' of https://git.kernel.org/.../matthias.bgg/linux Fabien Parent (1): drm/mediatek: Fix indentation in Makefile Mark Yacoub (2): drm/mediatek: Return from mtk_ovl_layer_config after mtk_ovl_layer_off drm/mediatek: Turn off Alpha bit when plane format has no alpha Pi-Hsun Shih (1): drm/mediatek: Check return value of mtk_drm_ddp_comp_for_plane. Yongqiang Niu (3): drm/mediatek: Fix can't get component for external display plane. drm/mediatek: Add gamma property according to hardware capability drm/mediatek: Add ctm property support drivers/gpu/drm/mediatek/Makefile | 2 +- drivers/gpu/drm/mediatek/mtk_disp_color.c | 7 +- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 76 ++- drivers/gpu/drm/mediatek/mtk_disp_rdma.c| 43 +++--- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 204 +++- drivers/gpu/drm/mediatek/mtk_drm_crtc.h | 2 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 184 + drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 56 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 86 +--- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 7 - drivers/gpu/drm/mediatek/mtk_drm_plane.c| 47 +++ drivers/gpu/drm/mediatek/mtk_drm_plane.h| 2 + drivers/soc/mediatek/mtk-cmdq-helper.c | 147 include/linux/mailbox/mtk-cmdq-mailbox.h| 11 ++ include/linux/soc/mediatek/mtk-cmdq.h | 53 15 files changed, 666 insertions(+), 261 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/dp_mst: Have DP_Tx send one msg at a time
[AMD Public Use]
Thanks for your time and hope you get well soon!
-Original Message-
From: Lyude Paul
Sent: Tuesday, January 14, 2020 1:59 AM
To: Lin, Wayne ; dri-devel@lists.freedesktop.org;
amd-...@lists.freedesktop.org
Cc: Kazlauskas, Nicholas ; Wentland, Harry
; Zuo, Jerry
Subject: Re: [PATCH] drm/dp_mst: Have DP_Tx send one msg at a time
Hey! Haven't taken a look at this patch yet but just wanted to let you know I'm
going to try to get to most of the stuff you've got pending for me. I came down
with a really nasty cold last week so sorry if you've had any other patches
waiting until now!
On Mon, 2020-01-13 at 17:36 +0800, Wayne Lin wrote:
> [Why]
> Noticed this while testing MST with the 4 ports MST hub from
> StarTech.com. Sometimes can't light up monitors normally and get the
> error message as 'sideband msg build failed'.
>
> Look into aux transactions, found out that source sometimes will send
> out another down request before receiving the down reply of the
> previous down request. On the other hand, in drm_dp_get_one_sb_msg(),
> current code doesn't handle the interleaved replies case. Hence,
> source can't build up message completely and can't light up monitors.
>
> [How]
> For good compatibility, enforce source to send out one down request at
> a time. Add a flag, is_waiting_for_dwn_reply, to determine if the
> source can send out a down request immediately or not.
>
> - Check the flag before calling process_single_down_tx_qlock to send
> out a msg
> - Set the flag when successfully send out a down request
> - Clear the flag when successfully build up a down reply
> - Clear the flag when find erros during sending out a down request
> - Clear the flag when find errors during building up a down reply
> - Clear the flag when timeout occurs during waiting for a down reply
> - Use drm_dp_mst_kick_tx() to try to send another down request in
> queue at the end of drm_dp_mst_wait_tx_reply() (attempt to send out
> messages in queue when errors occur)
>
> Cc: Lyude Paul
> Signed-off-by: Wayne Lin
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 14 --
> include/drm/drm_dp_mst_helper.h | 6 ++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 4395d5cc0645..3542af15387a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1195,6 +1195,8 @@ static int drm_dp_mst_wait_tx_reply(struct
> drm_dp_mst_branch *mstb,
> txmsg->state == DRM_DP_SIDEBAND_TX_SENT) {
> mstb->tx_slots[txmsg->seqno] = NULL;
> }
> + mgr->is_waiting_for_dwn_reply = false;
> +
> }
> out:
> if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) { @@
> -1204,6 +1206,7 @@ static int drm_dp_mst_wait_tx_reply(struct
> drm_dp_mst_branch *mstb,
> }
> mutex_unlock(&mgr->qlock);
>
> + drm_dp_mst_kick_tx(mgr);
> return ret;
> }
>
> @@ -2770,9 +2773,11 @@ static void process_single_down_tx_qlock(struct
> drm_dp_mst_topology_mgr *mgr)
> ret = process_single_tx_qlock(mgr, txmsg, false);
> if (ret == 1) {
> /* txmsg is sent it should be in the slots now */
> + mgr->is_waiting_for_dwn_reply = true;
> list_del(&txmsg->next);
> } else if (ret) {
> DRM_DEBUG_KMS("failed to send msg in q %d\n", ret);
> + mgr->is_waiting_for_dwn_reply = false;
> list_del(&txmsg->next);
> if (txmsg->seqno != -1)
> txmsg->dst->tx_slots[txmsg->seqno] = NULL; @@ -2812,7
> +2817,8 @@
> static void drm_dp_queue_down_tx(struct drm_dp_mst_topology_mgr *mgr,
> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> }
>
> - if (list_is_singular(&mgr->tx_msg_downq))
> + if (list_is_singular(&mgr->tx_msg_downq) &&
> + !mgr->is_waiting_for_dwn_reply)
> process_single_down_tx_qlock(mgr);
> mutex_unlock(&mgr->qlock);
> }
> @@ -3743,6 +3749,7 @@ static int drm_dp_mst_handle_down_rep(struct
> drm_dp_mst_topology_mgr *mgr)
> mutex_lock(&mgr->qlock);
> txmsg->state = DRM_DP_SIDEBAND_TX_RX;
> mstb->tx_slots[slot] = NULL;
> + mgr->is_waiting_for_dwn_reply = false;
> mutex_unlock(&mgr->qlock);
>
> wake_up_all(&mgr->tx_waitq);
> @@ -3752,6 +3759,9 @@ static int drm_dp_mst_handle_down_rep(struct
> drm_dp_mst_topology_mgr *mgr)
> no_msg:
> drm_dp_mst_topology_put_mstb(mstb);
> clear_down_rep_recv:
> + mutex_lock(&mgr->qlock);
> + mgr->is_waiting_for_dwn_reply = false;
> + mutex_unlock(&mgr->qlock);
> memset(&mgr->down_rep_recv, 0, sizeof(struct
> drm_dp_sideband_msg_rx));
>
> return 0;
> @@ -4591,7 +4601,7 @@ static void drm_dp_tx_work(struct work_struct *work)
> struct drm_dp_mst_topology_mgr *mgr = container_of(wor
Re: [Freedreno] [PATCH 2/2] drm/msm: Add MSM_WAIT_IOVA ioctl
On Mon, Jan 13, 2020 at 3:17 PM Rob Clark wrote:
>
> On Mon, Jan 13, 2020 at 2:55 PM Brian Ho wrote:
> >
> > On Mon, Jan 13, 2020 at 09:57:43AM -0800, Kristian Kristensen wrote:
> > > On Mon, Jan 13, 2020 at 8:25 AM Rob Clark wrote:
> > >
> > > > On Mon, Jan 13, 2020 at 7:37 AM Brian Ho wrote:
> > > > >
> > > > > Implements an ioctl to wait until a value at a given iova is greater
> > > > > than or equal to a supplied value.
> > > > >
> > > > > This will initially be used by turnip (open-source Vulkan driver for
> > > > > QC in mesa) for occlusion queries where the userspace driver can
> > > > > block on a query becoming available before continuing via
> > > > > vkGetQueryPoolResults.
> > > > >
> > > > > Signed-off-by: Brian Ho
> > > > > ---
> > > > > drivers/gpu/drm/msm/msm_drv.c | 63
> > > > > +--
> > > > > include/uapi/drm/msm_drm.h| 13
> > > > > 2 files changed, 74 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/msm_drv.c
> > > > b/drivers/gpu/drm/msm/msm_drv.c
> > > > > index c84f0a8b3f2c..dcc46874a5a2 100644
> > > > > --- a/drivers/gpu/drm/msm/msm_drv.c
> > > > > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > > > > @@ -36,10 +36,11 @@
> > > > > * MSM_GEM_INFO ioctl.
> > > > > * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to
> > > > > set/get
> > > > > * GEM object's debug name
> > > > > - * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
> > > > > + * - 1.5.0 - Add SUBMITQUEUE_QUERY ioctl
> > > > > + * - 1.6.0 - Add WAIT_IOVA ioctl
> > > > > */
> > > > > #define MSM_VERSION_MAJOR 1
> > > > > -#define MSM_VERSION_MINOR 5
> > > > > +#define MSM_VERSION_MINOR 6
> > > > > #define MSM_VERSION_PATCHLEVEL 0
> > > > >
> > > > > static const struct drm_mode_config_funcs mode_config_funcs = {
> > > > > @@ -952,6 +953,63 @@ static int msm_ioctl_submitqueue_close(struct
> > > > drm_device *dev, void *data,
> > > > > return msm_submitqueue_remove(file->driver_priv, id);
> > > > > }
> > > > >
> > > > > +static int msm_ioctl_wait_iova(struct drm_device *dev, void *data,
> > > > > + struct drm_file *file)
> > > > > +{
> > > > > + struct msm_drm_private *priv = dev->dev_private;
> > > > > + struct drm_gem_object *obj;
> > > > > + struct drm_msm_wait_iova *args = data;
> > > > > + ktime_t timeout = to_ktime(args->timeout);
> > > > > + unsigned long remaining_jiffies =
> > > > > timeout_to_jiffies(&timeout);
> > > > > + struct msm_gpu *gpu = priv->gpu;
> > > > > + void *base_vaddr;
> > > > > + uint64_t *vaddr;
> > > > > + int ret;
> > > > > +
> > > > > + if (args->pad)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + if (!gpu)
> > > > > + return 0;
> > > >
> > > > hmm, I'm not sure we should return zero in this case.. maybe -ENODEV?
> > > >
> > > > > +
> > > > > + obj = drm_gem_object_lookup(file, args->handle);
> > > > > + if (!obj)
> > > > > + return -ENOENT;
> > > > > +
> > > > > + base_vaddr = msm_gem_get_vaddr(obj);
> > > > > + if (IS_ERR(base_vaddr)) {
> > > > > + ret = PTR_ERR(base_vaddr);
> > > > > + goto err_put_gem_object;
> > > > > + }
> > > > > + if (args->offset + sizeof(*vaddr) > obj->size) {
> > > > > + ret = -EINVAL;
> > > > > + goto err_put_vaddr;
> > > > > + }
> > > > > +
> > > > > + vaddr = base_vaddr + args->offset;
> > > > > +
> > > > > + /* Assumes WC mapping */
> > > > > + ret = wait_event_interruptible_timeout(
> > > > > + gpu->event, *vaddr >= args->value,
> > > > remaining_jiffies);
> > > >
> > >
> > > This needs to do the awkward looking
> > >
> > > (int64_t)(*data - value) >= 0
> > >
> > > to properly handle the wraparound case.
> > >
> >
> > I think this comparison will run into issues if we allow for 64-bit
> > reference values. For example, if value is ULLONG_MAX, and *data
> > starts at 0 on the first comparison, we'll immediately return.
The comparison will have to account of the number of bits in the
serial number. The (int64_t) (*data - value) works for 64 bit unsigned
serial numbers, but for 32 bit serials as suggested below, we need to
cast to int32_t. It does work though, in the case where value is
ULLONG_MAX and and *data is 0, 0 - ULLONG_MAX is one, which is
correct. The serial numbers wrap around and the distance is computed
modulo 2^64. See
https://en.wikipedia.org/wiki/Serial_number_arithmetic.
> >
> > It's not too much of an issue in fence_completed (msm_fence.c), but
> > in this ioctl, *data can grow at an arbitrary rate. Are we concerned
> > about this?
> >
> > > > +
> > > > > + if (ret == 0) {
> > > > > + ret = -ETIMEDOUT;
> > > > > + goto err_put_vaddr;
> > > > > + } else if (ret == -ERESTARTSYS) {
> > > > > + goto
Re: [Freedreno] [PATCH] drm/msm: Add syncobj support.
On Tue, Jan 14, 2020 at 12:41 AM Jordan Crouse wrote:
>
> On Mon, Jan 13, 2020 at 09:25:57PM +0100, Bas Nieuwenhuizen wrote:
> > This
> >
> > 1) Enables core DRM syncobj support.
> > 2) Adds options to the submission ioctl to wait/signal syncobjs.
> >
> > Just like the wait fence fd, this does inline waits. Using the
> > scheduler would be nice but I believe it is out of scope for
> > this work.
> >
> > Support for timeline syncobjs is implemented and the interface
> > is ready for it, but I'm not enabling it yet until there is
> > some code for turnip to use it.
> >
> > The reset is mostly in there because in the presence of waiting
> > and signalling the same semaphores, resetting them after
> > signalling can become very annoying.
> >
> > Signed-off-by: Bas Nieuwenhuizen
> > ---
> >
> > Userspace code in
> >
> > https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2769
> >
> > drivers/gpu/drm/msm/msm_drv.c| 6 +-
> > drivers/gpu/drm/msm/msm_gem_submit.c | 241 ++-
> > include/uapi/drm/msm_drm.h | 22 ++-
> > 3 files changed, 265 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> > index c84f0a8b3f2c..ca36d6b21d8f 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.c
> > +++ b/drivers/gpu/drm/msm/msm_drv.c
> > @@ -37,9 +37,10 @@
> > * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
> > * GEM object's debug name
> > * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
> > + * - 1.6.0 - Syncobj support
> > */
> > #define MSM_VERSION_MAJOR1
> > -#define MSM_VERSION_MINOR5
> > +#define MSM_VERSION_MINOR6
> > #define MSM_VERSION_PATCHLEVEL 0
> >
> > static const struct drm_mode_config_funcs mode_config_funcs = {
> > @@ -988,7 +989,8 @@ static struct drm_driver msm_driver = {
> > .driver_features= DRIVER_GEM |
> > DRIVER_RENDER |
> > DRIVER_ATOMIC |
> > - DRIVER_MODESET,
> > + DRIVER_MODESET|
>
> A space before the | would be preferred.
Done.
>
> > + DRIVER_SYNCOBJ,
> > .open = msm_open,
> > .postclose = msm_postclose,
> > .lastclose = drm_fb_helper_lastclose,
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index be5327af16fa..9085229f88e0 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -8,7 +8,9 @@
> > #include
> > #include
> >
> > +#include
> > #include
> > +#include
> >
> > #include "msm_drv.h"
> > #include "msm_gpu.h"
> > @@ -394,6 +396,196 @@ static void submit_cleanup(struct msm_gem_submit
> > *submit)
> > ww_acquire_fini(&submit->ticket);
> > }
> >
> > +
> > +struct msm_submit_post_dep {
> > + struct drm_syncobj *syncobj;
> > + uint64_t point;
> > + struct dma_fence_chain *chain;
> > +};
> > +
> > +static int msm_wait_deps(struct drm_device *dev,
> > + struct drm_file *file,
> > + uint64_t in_syncobjs_addr,
> > + uint32_t nr_in_syncobjs,
> > + struct msm_ringbuffer *ring,
> > + struct drm_syncobj ***syncobjs)
> > +{
> > + struct drm_msm_gem_submit_syncobj *syncobj_descs;
> > + int ret = 0;
> > + uint32_t i, j;
> > +
> > + syncobj_descs = kmalloc_array(nr_in_syncobjs, sizeof(*syncobj_descs),
> > + GFP_KERNEL);
> > + if (!syncobj_descs)
> > + return -ENOMEM;
> > +
> We would want to watch out here for fuzzers and malicious actors that try to
> force an enormous memory allocation. It seems like we should be able to
> iteratively read each fences in the loop and skip this memory allocation.
>
> > + *syncobjs = kcalloc(nr_in_syncobjs, sizeof(**syncobjs), GFP_KERNEL);
> > + if (!syncobjs) {
> > + ret = -ENOMEM;
> > + goto out_syncobjs;
> > + }
>
> Alas no good way to skip this one. But it seems that syncobjs should only
> contain descriptors with MSM_SUBMIT_SYNCOBJ_RESET set. I'm not very familiar
> with how fences work so I'm not sure how common this path is. Would the same
> fuzzer or malicious actor be able to double the destruction by forcing a large
> allocation that doesn't even end up getting used?
In real usecases I expect MSM_SUBMIT_SYNCOBJ_RESET to be set for 50%+
of the entries and the number of entries to be < 10.
I can certainly start doing a copy_from_user per entry and save one of
the array. (I was under the impression that copy_from_user was
expensive but if it is not, okay)
Overall though, there is a real issue of wanting to delay all write
actions until we are sure the ioctl will succeed. That will mean we
need to have arrays that are on the order of a UINT32_MAX elements if
Re: [PATCH v2] drm/dp: Add function to parse EDID descriptors for adaptive sync limits
Hi Ville,
So the two major changes you would like to see here are:
use version_greate(edid) function
and make use of :
drm_for_each_detailed_block() instead of the for loop.
But this function does not parse the monitor range yet so
you are suggesting modifying that dmr helper function as well?
Manasi
On Fri, Jan 10, 2020 at 03:17:43PM -0800, Manasi Navare wrote:
> On Thu, Jan 09, 2020 at 03:08:52PM +0200, Ville Syrjälä wrote:
> > On Tue, Jan 07, 2020 at 04:32:08PM -0800, Manasi Navare wrote:
> > > Adaptive Sync is a VESA feature so add a DRM core helper to parse
> > > the EDID's detailed descritors to obtain the adaptive sync monitor range.
> > > Store this info as part fo drm_display_info so it can be used
> > > across all drivers.
> > > This part of the code is stripped out of amdgpu's function
> > > amdgpu_dm_update_freesync_caps() to make it generic and be used
> > > across all DRM drivers
> > >
> > > v2:
> > > * Change vmin and vmax to use u8 (Ville)
> > > * Dont store pixel clock since that is just a max dotclock
> > > and not related to VRR mode (Manasi)
> > >
> > > Cc: Ville Syrjälä
> > > Cc: Harry Wentland
> > > Cc: Clinton A Taylor
> > > Cc: Nicholas Kazlauskas
> > > Signed-off-by: Manasi Navare
> > > ---
> > > drivers/gpu/drm/drm_edid.c | 51 +
> > > include/drm/drm_connector.h | 22
> > > include/drm/drm_edid.h | 2 ++
> > > 3 files changed, 75 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 99769d6c9f84..52781a0e708b 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -4880,6 +4880,54 @@ static void drm_parse_cea_ext(struct drm_connector
> > > *connector,
> > > }
> > > }
> > >
> > > +void drm_get_adaptive_sync_limits(struct drm_connector *connector,
> > > + const struct edid *edid)
> > > +{
> > > + struct drm_display_info *info = &connector->display_info;
> > > + const struct detailed_timing *timing;
> > > + const struct detailed_non_pixel *data;
> > > + const struct detailed_data_monitor_range *range;
> >
> > Needlessly wide scope for everything above.
>
> Okay will move inside the for_each loop ?
>
> >
> > > + int i;
> > > +
> > > + /*
> > > + * Restrict Adaptive Sync only for dp and edp
> > > + */
> > > + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> > > + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> > > + return;
> > > +
> > > + if (edid->version <= 1 && !(edid->version == 1 && edid->revision > 1))
> > > + return;
> >
> > if (!version_greater(...))
> > return;
>
> You mean simplify the condition or use define a function version_greater() to
> check this condition?
>
> >
> > > +
> > > + for (i = 0; i < 4; i++) {
> >
> > This should probably use for_each_detailed_block()
> >
>
>
> Ok yes will use for_each_detailed_block
>
> Manasi
>
>
> > > + timing = &edid->detailed_timings[i];
> > > + data= &timing->data.other_data;
> > > + range = &data->data.range;
> > > + /*
> > > + * Check if monitor has continuous frequency mode
> > > + */
> > > + if (data->type != EDID_DETAIL_MONITOR_RANGE)
> > > + continue;
> > > + /*
> > > + * Check for flag range limits only. If flag == 1 then
> > > + * no additional timing information provided.
> > > + * Default GTF, GTF Secondary curve and CVT are not
> > > + * supported
> > > + */
> > > + if (range->flags != 1)
> > > + continue;
> > > +
> > > + info->adaptive_sync.min_vfreq = range->min_vfreq;
> > > + info->adaptive_sync.max_vfreq = range->max_vfreq;
> > > +
> > > + DRM_DEBUG_KMS("Adaptive Sync refresh rate range is %d Hz - %d
> > > Hz\n",
> > > + info->adaptive_sync.min_vfreq,
> > > + info->adaptive_sync.max_vfreq);
> > > + break;
> > > + }
> > > +}
> > > +EXPORT_SYMBOL(drm_get_adaptive_sync_limits);
> > > +
> > > /* A connector has no EDID information, so we've got no EDID to compute
> > > quirks from. Reset
> > > * all of the values which would have been set from EDID
> > > */
> > > @@ -4901,6 +4949,7 @@ drm_reset_display_info(struct drm_connector
> > > *connector)
> > > memset(&info->hdmi, 0, sizeof(info->hdmi));
> > >
> > > info->non_desktop = 0;
> > > + memset(&info->adaptive_sync, 0, sizeof(info->adaptive_sync));
> > > }
> > >
> > > u32 drm_add_display_info(struct drm_connector *connector, const struct
> > > edid *edid)
> > > @@ -4916,6 +4965,8 @@ u32 drm_add_display_info(struct drm_connector
> > > *connector, const struct edid *edi
> > >
> > > info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP);
> > >
> > > + drm_get_adaptive_sync_limits(connector, edid);
> > > +
> > > DRM_DEBUG_KMS("non_desktop set to %d\n", info-
Re: [Freedreno] [PATCH] drm/msm: Add syncobj support.
On Mon, Jan 13, 2020 at 09:25:57PM +0100, Bas Nieuwenhuizen wrote:
> This
>
> 1) Enables core DRM syncobj support.
> 2) Adds options to the submission ioctl to wait/signal syncobjs.
>
> Just like the wait fence fd, this does inline waits. Using the
> scheduler would be nice but I believe it is out of scope for
> this work.
>
> Support for timeline syncobjs is implemented and the interface
> is ready for it, but I'm not enabling it yet until there is
> some code for turnip to use it.
>
> The reset is mostly in there because in the presence of waiting
> and signalling the same semaphores, resetting them after
> signalling can become very annoying.
>
> Signed-off-by: Bas Nieuwenhuizen
> ---
>
> Userspace code in
>
> https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2769
>
> drivers/gpu/drm/msm/msm_drv.c| 6 +-
> drivers/gpu/drm/msm/msm_gem_submit.c | 241 ++-
> include/uapi/drm/msm_drm.h | 22 ++-
> 3 files changed, 265 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index c84f0a8b3f2c..ca36d6b21d8f 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -37,9 +37,10 @@
> * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
> * GEM object's debug name
> * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
> + * - 1.6.0 - Syncobj support
> */
> #define MSM_VERSION_MAJOR1
> -#define MSM_VERSION_MINOR5
> +#define MSM_VERSION_MINOR6
> #define MSM_VERSION_PATCHLEVEL 0
>
> static const struct drm_mode_config_funcs mode_config_funcs = {
> @@ -988,7 +989,8 @@ static struct drm_driver msm_driver = {
> .driver_features= DRIVER_GEM |
> DRIVER_RENDER |
> DRIVER_ATOMIC |
> - DRIVER_MODESET,
> + DRIVER_MODESET|
A space before the | would be preferred.
> + DRIVER_SYNCOBJ,
> .open = msm_open,
> .postclose = msm_postclose,
> .lastclose = drm_fb_helper_lastclose,
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index be5327af16fa..9085229f88e0 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -8,7 +8,9 @@
> #include
> #include
>
> +#include
> #include
> +#include
>
> #include "msm_drv.h"
> #include "msm_gpu.h"
> @@ -394,6 +396,196 @@ static void submit_cleanup(struct msm_gem_submit
> *submit)
> ww_acquire_fini(&submit->ticket);
> }
>
> +
> +struct msm_submit_post_dep {
> + struct drm_syncobj *syncobj;
> + uint64_t point;
> + struct dma_fence_chain *chain;
> +};
> +
> +static int msm_wait_deps(struct drm_device *dev,
> + struct drm_file *file,
> + uint64_t in_syncobjs_addr,
> + uint32_t nr_in_syncobjs,
> + struct msm_ringbuffer *ring,
> + struct drm_syncobj ***syncobjs)
> +{
> + struct drm_msm_gem_submit_syncobj *syncobj_descs;
> + int ret = 0;
> + uint32_t i, j;
> +
> + syncobj_descs = kmalloc_array(nr_in_syncobjs, sizeof(*syncobj_descs),
> + GFP_KERNEL);
> + if (!syncobj_descs)
> + return -ENOMEM;
> +
We would want to watch out here for fuzzers and malicious actors that try to
force an enormous memory allocation. It seems like we should be able to
iteratively read each fences in the loop and skip this memory allocation.
> + *syncobjs = kcalloc(nr_in_syncobjs, sizeof(**syncobjs), GFP_KERNEL);
> + if (!syncobjs) {
> + ret = -ENOMEM;
> + goto out_syncobjs;
> + }
Alas no good way to skip this one. But it seems that syncobjs should only
contain descriptors with MSM_SUBMIT_SYNCOBJ_RESET set. I'm not very familiar
with how fences work so I'm not sure how common this path is. Would the same
fuzzer or malicious actor be able to double the destruction by forcing a large
allocation that doesn't even end up getting used?
> + if (copy_from_user(syncobj_descs, u64_to_user_ptr(in_syncobjs_addr),
> +nr_in_syncobjs * sizeof(*syncobj_descs))) {
> + ret = -EFAULT;
> + goto out_syncobjs;
> + }
> +
> + for (i = 0; i < nr_in_syncobjs; ++i) {
> + struct dma_fence *fence;
> +
> + if (syncobj_descs[i].point &&
> + !drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) {
> + ret = -EOPNOTSUPP;
You should be able to just break; here.
> + goto out_syncobjs;
> + }
> +
> + if (syncobj_descs[i].flags & ~MSM_SUBMIT_SYNCOBJ_FLAGS) {
> + ret = -EINVAL;
and here.
> + goto out_syncobjs;
> +
Re: [PATCH v5 5/5] MAINTAINERS: add entry for tidss
Jyri Sarha wrote on Fri [2019-Dec-20 17:55:13 +0200]: > Add entry for tidss DRM driver. > > Version history: > > v2: no change > > v3: - Move tidss entry after omapdrm > - Add "T: git git://anongit.freedesktop.org/drm/drm-misc" > > v4: no change > > v5: no change > > Signed-off-by: Jyri Sarha Reviewed-by: Benoit Parrot ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/5] dt-bindings: display: ti, am65x-dss: Add dt-schema yaml binding
Jyri Sarha wrote on Fri [2019-Dec-20 17:55:10 +0200]: > Add dt-schema yaml bindig for AM65x DSS, AM65x version TI Keystone > Display SubSystem. > > Version history: > > v2: no change > > v3: - Add ports node > - use allOf in ti,am65x-oldi-io-ctrl to add both $ref and maxItems > - Add includes to dts example > - reindent dts example > > v4: - Add descriptions to reg and clocks properties > - Remove minItems when its value is the same as maxItems value > > v5: - itemize reg and clocks properties' descriptions > > Signed-off-by: Jyri Sarha Reviewed-by: Benoit Parrot ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/5] dt-bindings: display: ti,k2g-dss: Add dt-schema yaml binding
Jyri Sarha wrote on Fri [2019-Dec-20 17:55:09 +0200]: > Add dt-schema yaml bindig for K2G DSS, an ultra-light version of TI > Keystone Display SubSystem. > > Version history: > > v2: no change > > v3: - Add ports node > - Add includes to dts example > - reindent dts example > > v4: - Add descriptions to reg and clocks properties > - Remove minItems when its value is the same as maxItems value > - Remove ports node > > v5: - itemize reg and clocks properties' descriptions > > Signed-off-by: Jyri Sarha Reviewed-by: Benoit Parrot ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 4/5] drm/tidss: New driver for TI Keystone platform Display SubSystem
Jyri Sarha wrote on Fri [2019-Dec-20 17:55:12 +0200]: > This patch adds a new DRM driver for Texas Instruments DSS IPs used on > Texas Instruments Keystone K2G, AM65x, and J721e SoCs. The new DSS IP is > a major change to the older DSS IP versions, which are supported by > the omapdrm driver. While on higher level the Keystone DSS resembles > the older DSS versions, the registers are completely different and the > internal pipelines differ a lot. > > DSS IP found on K2G is an "ultra-light" version, and has only a single > plane and a single output. The Keystone 3 DSS IPs are found on AM65x > and J721E SoCs. AM65x DSS has two video ports, one full video plane, > and another "lite" plane without scaling support. J721E has 4 video > ports, 2 video planes and 2 lite planes. AM65x DSS has also integrated > OLDI (LVDS) output. > > Version history: > > v2: - rebased on top of drm-next-2019-11-27 > - sort all include lines in all files > - remove all include > - remove select "select VIDEOMODE_HELPERS" > - call dispc_vp_setup() later in tidss_crtc_atomic_flush() (there is no > to call it in new modeset case as it is also called in vp_enable()) > - change probe sequence and drm_device allocation (follow example in > drm_drv.c) > - use __maybe_unused instead of #ifdef for pm functions > - remove "struct drm_fbdev_cma *fbdev;" from driver data > - check panel connector type before connecting it > > v3: no change > > v4: no change > > v5: - remove fifo underflow irq handling, it is not an error and > it should be used for debug purposes only > - memory tuning, prefetch plane fifo up to high-threshold value to > minimize possibility of underflows. > > Co-developed-by: Tomi Valkeinen > Signed-off-by: Jyri Sarha Reviewed-by: Benoit Parrot ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 3/5] dt-bindings: display: ti, j721e-dss: Add dt-schema yaml binding
Jyri Sarha wrote on Fri [2019-Dec-20 17:55:11 +0200]: > Add dt-schema yaml bindig for J721E DSS, J721E version TI Keystone > Display SubSystem. > > Version history: > > v2: no change > > v3: - reg-names: "wp" -> "wb" > - Add ports node > - Add includes to dts example > - reindent dts example > > v4: - Add descriptions to reg, clocks, and interrups properties > - Remove minItems when its value is the same as maxItems value > > v5: - itemize reg, clocks and interrupts properties' descriptions > - there is no "vp" reg-name, only "wb" for write back > > Signed-off-by: Jyri Sarha Reviewed-by: Benoit Parrot ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: Fix error about comments within a comment block
My compiler yells:
.../drivers/gpu/drm/msm/adreno/adreno_gpu.c:69:27:
error: '/*' within block comment [-Werror,-Wcomment]
Let's fix.
Fixes: 6a0dea02c2c4 ("drm/msm: support firmware-name for zap fw (v2)")
Signed-off-by: Douglas Anderson
---
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index c146c3b8f52b..7fd29829b2fa 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -67,7 +67,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
char *fwname,
*
* If the firmware-name property is found, we bypass the
* adreno_request_fw() mechanism, because we don't need to handle
-* the /lib/firmware/qcom/* vs /lib/firmware/* case.
+* the /lib/firmware/qcom/... vs /lib/firmware/... case.
*
* If the firmware-name property is not found, for backwards
* compatibility we fall back to the fwname from the gpulist
--
2.25.0.rc1.283.g88dfdc4193-goog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] dt-bindings: display: add sc7180 panel variant
Hi,
On Tue, Jan 07, 2020 at 04:59:56PM +0530, Harigovindan P wrote:
> Subject: dt-bindings: display: add sc7180 panel variant
>
> Add a compatible string to support sc7180 panel version.
The sc7180 is a SoC, I suppose you are referring to the sc7180-idp, a
board based on this SoC. But even with the correct board name, this
isn't a good commit message. The board is irrelevant here, the display
could be used on dozens of other boards.
Apparently the panel is the Visionix RM69299, please use this instead
of 'sc7180'.
Thanks
Matthias
> Changes in v1:
> -Added a compatible string to support sc7180 panel version.
> Changes in v2:
> -Removed unwanted properties from description.
> -Creating source files without execute permissions(Rob Herring).
>
> Signed-off-by: Harigovindan P
> ---
> .../bindings/display/visionox,rm69299.txt | 48
> ++
> 1 file changed, 48 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/visionox,rm69299.txt
>
> diff --git a/Documentation/devicetree/bindings/display/visionox,rm69299.txt
> b/Documentation/devicetree/bindings/display/visionox,rm69299.txt
> new file mode 100644
> index 000..d7bbd5f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/visionox,rm69299.txt
> @@ -0,0 +1,48 @@
> +Visionox model RM69299 DSI display driver
> +
> +The Visionox RM69299 is a generic display driver, currently only configured
> +for use in the 1080p display on the Qualcomm SC7180 MTP board.
> +
> +Required properties:
> +- compatible: should be "visionox,rm69299-1080p-display"
> +- vdda-supply: phandle of the regulator that provides the supply voltage
> + Power IC supply
> +- vdd3p3-supply: phandle of the regulator that provides the supply voltage
> + Power IC supply
> +- reset-gpios: phandle of gpio for reset line
> + This should be 8mA, gpio can be configured using mux, pinctrl,
> pinctrl-names
> + (active low)
> +- ports: This device has one video port driven by one DSI. Their connections
> + are modeled using the OF graph bindings specified in
> + Documentation/devicetree/bindings/graph.txt.
> + - port@0: DSI input port driven by master DSI
> +
> +Example:
> +
> + dsi@ae94000 {
> + panel@0 {
> + compatible = "visionox,rm69299-1080p-display";
> + reg = <0>;
> +
> + vdda-supply = <&src_pp1800_l8c>;
> + vdd3p3-supply = <&src_pp2800_l18a>;
> +
> + pinctrl-names = "default", "suspend";
> + pinctrl-0 = <&disp_pins_default>;
> + pinctrl-1 = <&disp_pins_default>;
> +
> + reset-gpios = <&pm6150l_gpios 3 0>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + panel0_in: endpoint {
> + remote-endpoint = <&dsi0_out>;
> + };
> + };
> + };
> + };
> + };
> --
> 2.7.4
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm: Add syncobj support.
This
1) Enables core DRM syncobj support.
2) Adds options to the submission ioctl to wait/signal syncobjs.
Just like the wait fence fd, this does inline waits. Using the
scheduler would be nice but I believe it is out of scope for
this work.
Support for timeline syncobjs is implemented and the interface
is ready for it, but I'm not enabling it yet until there is
some code for turnip to use it.
The reset is mostly in there because in the presence of waiting
and signalling the same semaphores, resetting them after
signalling can become very annoying.
Signed-off-by: Bas Nieuwenhuizen
---
Userspace code in
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/2769
drivers/gpu/drm/msm/msm_drv.c| 6 +-
drivers/gpu/drm/msm/msm_gem_submit.c | 241 ++-
include/uapi/drm/msm_drm.h | 22 ++-
3 files changed, 265 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index c84f0a8b3f2c..ca36d6b21d8f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -37,9 +37,10 @@
* - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
* GEM object's debug name
* - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
+ * - 1.6.0 - Syncobj support
*/
#define MSM_VERSION_MAJOR 1
-#define MSM_VERSION_MINOR 5
+#define MSM_VERSION_MINOR 6
#define MSM_VERSION_PATCHLEVEL 0
static const struct drm_mode_config_funcs mode_config_funcs = {
@@ -988,7 +989,8 @@ static struct drm_driver msm_driver = {
.driver_features= DRIVER_GEM |
DRIVER_RENDER |
DRIVER_ATOMIC |
- DRIVER_MODESET,
+ DRIVER_MODESET|
+ DRIVER_SYNCOBJ,
.open = msm_open,
.postclose = msm_postclose,
.lastclose = drm_fb_helper_lastclose,
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index be5327af16fa..9085229f88e0 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -8,7 +8,9 @@
#include
#include
+#include
#include
+#include
#include "msm_drv.h"
#include "msm_gpu.h"
@@ -394,6 +396,196 @@ static void submit_cleanup(struct msm_gem_submit *submit)
ww_acquire_fini(&submit->ticket);
}
+
+struct msm_submit_post_dep {
+ struct drm_syncobj *syncobj;
+ uint64_t point;
+ struct dma_fence_chain *chain;
+};
+
+static int msm_wait_deps(struct drm_device *dev,
+ struct drm_file *file,
+ uint64_t in_syncobjs_addr,
+ uint32_t nr_in_syncobjs,
+ struct msm_ringbuffer *ring,
+ struct drm_syncobj ***syncobjs)
+{
+ struct drm_msm_gem_submit_syncobj *syncobj_descs;
+ int ret = 0;
+ uint32_t i, j;
+
+ syncobj_descs = kmalloc_array(nr_in_syncobjs, sizeof(*syncobj_descs),
+ GFP_KERNEL);
+ if (!syncobj_descs)
+ return -ENOMEM;
+
+ *syncobjs = kcalloc(nr_in_syncobjs, sizeof(**syncobjs), GFP_KERNEL);
+ if (!syncobjs) {
+ ret = -ENOMEM;
+ goto out_syncobjs;
+ }
+
+ if (copy_from_user(syncobj_descs, u64_to_user_ptr(in_syncobjs_addr),
+ nr_in_syncobjs * sizeof(*syncobj_descs))) {
+ ret = -EFAULT;
+ goto out_syncobjs;
+ }
+
+ for (i = 0; i < nr_in_syncobjs; ++i) {
+ struct dma_fence *fence;
+
+ if (syncobj_descs[i].point &&
+ !drm_core_check_feature(dev, DRIVER_SYNCOBJ_TIMELINE)) {
+ ret = -EOPNOTSUPP;
+ goto out_syncobjs;
+ }
+
+ if (syncobj_descs[i].flags & ~MSM_SUBMIT_SYNCOBJ_FLAGS) {
+ ret = -EINVAL;
+ goto out_syncobjs;
+ }
+
+ ret = drm_syncobj_find_fence(file, syncobj_descs[i].handle,
+syncobj_descs[i].point, 0, &fence);
+ if (ret)
+ goto out_syncobjs;
+
+ if (!dma_fence_match_context(fence, ring->fctx->context))
+ ret = dma_fence_wait(fence, true);
+
+ dma_fence_put(fence);
+ if (ret)
+ goto out_syncobjs;
+
+ if (syncobj_descs[i].flags & MSM_SUBMIT_SYNCOBJ_RESET) {
+ (*syncobjs)[i] =
+ drm_syncobj_find(file, syncobj_descs[i].handle);
+ if (!(*syncobjs)[i]) {
+ ret = -EINVAL;
+ goto out_syncobjs;
+ }
+ }
+ }
+
+out_syncobjs:
+ if (ret && *
Re: [PATCH 17/23] drm/radeon: Convert to CRTC VBLANK callbacks
On Fri, Jan 10, 2020 at 4:22 AM Thomas Zimmermann wrote:
>
> VBLANK callbacks in struct drm_driver are deprecated in favor of
> their equivalents in struct drm_crtc_funcs. Convert radeon over.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Alex Deucher
> ---
> drivers/gpu/drm/radeon/radeon_display.c | 12 --
> drivers/gpu/drm/radeon/radeon_drv.c | 7 --
> drivers/gpu/drm/radeon/radeon_kms.c | 29 ++---
> 3 files changed, 26 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> index 7187158b9963..9116975b6eb9 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -45,6 +45,10 @@
> #include "atom.h"
> #include "radeon.h"
>
> +u32 radeon_get_vblank_counter_kms(struct drm_crtc *crtc);
> +int radeon_enable_vblank_kms(struct drm_crtc *crtc);
> +void radeon_disable_vblank_kms(struct drm_crtc *crtc);
> +
> static void avivo_crtc_load_lut(struct drm_crtc *crtc)
> {
> struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> @@ -458,7 +462,7 @@ static void radeon_flip_work_func(struct work_struct
> *__work)
> (DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_IN_VBLANK) &&
> (!ASIC_IS_AVIVO(rdev) ||
> ((int) (work->target_vblank -
> - dev->driver->get_vblank_counter(dev, work->crtc_id)) > 0)))
> + crtc->funcs->get_vblank_counter(crtc)) > 0)))
> usleep_range(1000, 2000);
>
> /* We borrow the event spin lock for protecting flip_status */
> @@ -574,7 +578,7 @@ static int radeon_crtc_page_flip_target(struct drm_crtc
> *crtc,
> }
> work->base = base;
> work->target_vblank = target - (uint32_t)drm_crtc_vblank_count(crtc) +
> - dev->driver->get_vblank_counter(dev, work->crtc_id);
> + crtc->funcs->get_vblank_counter(crtc);
>
> /* We borrow the event spin lock for protecting flip_work */
> spin_lock_irqsave(&crtc->dev->event_lock, flags);
> @@ -666,6 +670,10 @@ static const struct drm_crtc_funcs radeon_crtc_funcs = {
> .set_config = radeon_crtc_set_config,
> .destroy = radeon_crtc_destroy,
> .page_flip_target = radeon_crtc_page_flip_target,
> + .get_vblank_counter = radeon_get_vblank_counter_kms,
> + .enable_vblank = radeon_enable_vblank_kms,
> + .disable_vblank = radeon_disable_vblank_kms,
> + .get_vblank_timestamp = drm_crtc_calc_vbltimestamp_from_scanoutpos,
> };
>
> static void radeon_crtc_init(struct drm_device *dev, int index)
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index 1f597f166bff..49ce2e7d5f9e 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -119,9 +119,6 @@ void radeon_driver_postclose_kms(struct drm_device *dev,
> int radeon_suspend_kms(struct drm_device *dev, bool suspend,
>bool fbcon, bool freeze);
> int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon);
> -u32 radeon_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
> -int radeon_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
> -void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
> void radeon_driver_irq_preinstall_kms(struct drm_device *dev);
> int radeon_driver_irq_postinstall_kms(struct drm_device *dev);
> void radeon_driver_irq_uninstall_kms(struct drm_device *dev);
> @@ -571,10 +568,6 @@ static struct drm_driver kms_driver = {
> .postclose = radeon_driver_postclose_kms,
> .lastclose = radeon_driver_lastclose_kms,
> .unload = radeon_driver_unload_kms,
> - .get_vblank_counter = radeon_get_vblank_counter_kms,
> - .enable_vblank = radeon_enable_vblank_kms,
> - .disable_vblank = radeon_disable_vblank_kms,
> - .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> .irq_preinstall = radeon_driver_irq_preinstall_kms,
> .irq_postinstall = radeon_driver_irq_postinstall_kms,
> .irq_uninstall = radeon_driver_irq_uninstall_kms,
> diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
> b/drivers/gpu/drm/radeon/radeon_kms.c
> index d24f23a81656..cab891f86dc0 100644
> --- a/drivers/gpu/drm/radeon/radeon_kms.c
> +++ b/drivers/gpu/drm/radeon/radeon_kms.c
> @@ -739,14 +739,15 @@ void radeon_driver_postclose_kms(struct drm_device *dev,
> /**
> * radeon_get_vblank_counter_kms - get frame count
> *
> - * @dev: drm dev pointer
> - * @pipe: crtc to get the frame count from
> + * @crtc: crtc to get the frame count from
> *
> * Gets the frame count on the requested crtc (all asics).
> * Returns frame count on success, -EINVAL on failure.
> */
> -u32 radeon_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe)
> +u32 radeon_get_vblank_counter_kms(struct drm_crtc *crtc)
> {
> +
Re: [PATCH 12/23] drm/amdgpu: Convert to CRTC VBLANK callbacks
On Fri, Jan 10, 2020 at 4:22 AM Thomas Zimmermann wrote:
>
> VBLANK callbacks in struct drm_driver are deprecated in favor of
> their equivalents in struct drm_crtc_funcs. Convert amdgpu over.
I think I'd prefer to just update the signatures of the relevant
functions rather than wrapping them.
Alex
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 3 +++
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 24 +++
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 4
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 4
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 4
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 4
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 4
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 ++
> 9 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 81a531b652aa..c1262ab588c9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1197,6 +1197,9 @@ int amdgpu_device_resume(struct drm_device *dev, bool
> fbcon);
> u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
> int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
> void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
> +u32 amdgpu_crtc_get_vblank_counter(struct drm_crtc *crtc);
> +int amdgpu_crtc_enable_vblank(struct drm_crtc *crtc);
> +void amdgpu_crtc_disable_vblank(struct drm_crtc *crtc);
> long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd,
> unsigned long arg);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 0749285dd1c7..9baa1ddf8693 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1377,10 +1377,6 @@ static struct drm_driver kms_driver = {
> .postclose = amdgpu_driver_postclose_kms,
> .lastclose = amdgpu_driver_lastclose_kms,
> .unload = amdgpu_driver_unload_kms,
> - .get_vblank_counter = amdgpu_get_vblank_counter_kms,
> - .enable_vblank = amdgpu_enable_vblank_kms,
> - .disable_vblank = amdgpu_disable_vblank_kms,
> - .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> .irq_handler = amdgpu_irq_handler,
> .ioctls = amdgpu_ioctls_kms,
> .gem_free_object_unlocked = amdgpu_gem_object_free,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 60591dbc2097..efe4671fb032 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1174,6 +1174,14 @@ u32 amdgpu_get_vblank_counter_kms(struct drm_device
> *dev, unsigned int pipe)
> return count;
> }
>
> +u32 amdgpu_crtc_get_vblank_counter(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + unsigned int pipe = crtc->index;
> +
> + return amdgpu_get_vblank_counter_kms(dev, pipe);
> +}
> +
> /**
> * amdgpu_enable_vblank_kms - enable vblank interrupt
> *
> @@ -1191,6 +1199,14 @@ int amdgpu_enable_vblank_kms(struct drm_device *dev,
> unsigned int pipe)
> return amdgpu_irq_get(adev, &adev->crtc_irq, idx);
> }
>
> +int amdgpu_crtc_enable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + unsigned int pipe = crtc->index;
> +
> + return amdgpu_enable_vblank_kms(dev, pipe);
> +}
> +
> /**
> * amdgpu_disable_vblank_kms - disable vblank interrupt
> *
> @@ -1207,6 +1223,14 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev,
> unsigned int pipe)
> amdgpu_irq_put(adev, &adev->crtc_irq, idx);
> }
>
> +void amdgpu_crtc_disable_vblank(struct drm_crtc *crtc)
> +{
> + struct drm_device *dev = crtc->dev;
> + unsigned int pipe = crtc->index;
> +
> + amdgpu_disable_vblank_kms(dev, pipe);
> +}
> +
> const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
> DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl,
> DRM_AUTH|DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl,
> DRM_AUTH|DRM_RENDER_ALLOW),
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index bdc1e0f036d4..8e62f46f0bfd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2494,6 +2494,10 @@ static const struct drm_crtc_funcs
> dce_v10_0_crtc_funcs = {
> .set_config = amdgpu_display_crtc_set_config,
> .destroy = dce_v10_0_crtc_destroy,
> .page_flip_target = amdgpu_display_crtc_page_flip_target,
> + .get_vblank_counter = amdgpu_crtc_get_vblank_counter,
> + .enable_vblank = amdgpu_crtc_enable_vblank,
> +
Re: [Intel-gfx] [PATCH 1/6] drm/print: introduce new struct drm_device based WARN* macros
Hi Pankaj.
On Mon, Jan 13, 2020 at 05:25:52PM +0530, Pankaj Bharadiya wrote:
> Add new struct drm_device based WARN* macros. These are modeled after
> the core kernel device based WARN* macros. These would be preferred
> over the regular WARN* macros, where possible.
>
> These macros include device information in the backtrace, so we know
> what device the warnings originate from.
>
> Knowing the device specific information in the backtrace would be
> helpful in development all around.
>
> Signed-off-by: Pankaj Bharadiya
> ---
> include/drm/drm_print.h | 30 ++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 8f99d389792d..61a7224e697d 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -553,4 +553,34 @@ void __drm_err(const char *format, ...);
> #define DRM_DEBUG_PRIME_RATELIMITED(fmt, ...)
> \
> DRM_DEV_DEBUG_PRIME_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
>
> +/*
> + * struct drm_device based WARNs
> + *
> + *
> + * drm_WARN*() acts like WARN*(), but with the key difference of
> + * using device specific information so that we know from which device
> + * warning is originating from.
> + *
> + * Prefer drm_device based dev_WARN* over regular WARN*
> + */
...
> + drm_WARN_ONCE((drm), (x), "%s", \
> + "drm_WARN_ON_ONCE(" __stringify(x) ")")
> +
> #endif /* DRM_PRINT_H_ */
As this is the now recommend way to use WARN() and friends, can we have
this moved upward in this file.
So we at some not-so-distant-future can have all the legacy logging in
the bottom of the file, and all the shiny new stuff at the top.
So people browsing the file see the new stuff before the legacy stuff.
Not a big deal - we can always move it later if we are inclined to do
so.
Very good that you follow-up with some actual usage.
This is a good way to verify that you got it right.
Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 05/23] drm/radeon: Convert to struct drm_crtc_helper_funcs.get_scanout_position()
On Fri, Jan 10, 2020 at 4:22 AM Thomas Zimmermann wrote:
>
> The callback struct drm_driver.get_scanout_position() is deprecated in
> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert
> radeon over.
>
I'd prefer to just change the signature of
radeon_get_crtc_scanoutpos() to match the new API.
Alex
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/radeon/atombios_crtc.c | 1 +
> drivers/gpu/drm/radeon/radeon_display.c | 13 +
> drivers/gpu/drm/radeon/radeon_drv.c | 11 ---
> drivers/gpu/drm/radeon/radeon_legacy_crtc.c | 3 ++-
> drivers/gpu/drm/radeon/radeon_mode.h| 6 ++
> 5 files changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c
> b/drivers/gpu/drm/radeon/atombios_crtc.c
> index da2c9e295408..447d74b78f19 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -2232,6 +2232,7 @@ static const struct drm_crtc_helper_funcs
> atombios_helper_funcs = {
> .prepare = atombios_crtc_prepare,
> .commit = atombios_crtc_commit,
> .disable = atombios_crtc_disable,
> + .get_scanout_position = radeon_get_crtc_scanout_position,
> };
>
> void radeon_atombios_init_crtc(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> index 962575e27cde..7187158b9963 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -1978,3 +1978,16 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev,
> unsigned int pipe,
>
> return ret;
> }
> +
> +bool
> +radeon_get_crtc_scanout_position(struct drm_crtc *crtc,
> +bool in_vblank_irq, int *vpos, int *hpos,
> +ktime_t *stime, ktime_t *etime,
> +const struct drm_display_mode *mode)
> +{
> + struct drm_device *dev = crtc->dev;
> + unsigned int pipe = crtc->index;
> +
> + return radeon_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
> + stime, etime, mode);
> +}
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c
> b/drivers/gpu/drm/radeon/radeon_drv.c
> index fd74e2611185..1f597f166bff 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -563,16 +563,6 @@ static const struct file_operations
> radeon_driver_kms_fops = {
> #endif
> };
>
> -static bool
> -radeon_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
> -bool in_vblank_irq, int *vpos, int *hpos,
> -ktime_t *stime, ktime_t *etime,
> -const struct drm_display_mode *mode)
> -{
> - return radeon_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
> - stime, etime, mode);
> -}
> -
> static struct drm_driver kms_driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_GEM | DRIVER_RENDER,
> @@ -585,7 +575,6 @@ static struct drm_driver kms_driver = {
> .enable_vblank = radeon_enable_vblank_kms,
> .disable_vblank = radeon_disable_vblank_kms,
> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> - .get_scanout_position = radeon_get_crtc_scanout_position,
> .irq_preinstall = radeon_driver_irq_preinstall_kms,
> .irq_postinstall = radeon_driver_irq_postinstall_kms,
> .irq_uninstall = radeon_driver_irq_uninstall_kms,
> diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
> b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
> index a1985a552794..8817fd033cd0 100644
> --- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
> +++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
> @@ -,7 +,8 @@ static const struct drm_crtc_helper_funcs
> legacy_helper_funcs = {
> .mode_set_base_atomic = radeon_crtc_set_base_atomic,
> .prepare = radeon_crtc_prepare,
> .commit = radeon_crtc_commit,
> - .disable = radeon_crtc_disable
> + .disable = radeon_crtc_disable,
> + .get_scanout_position = radeon_get_crtc_scanout_position,
> };
>
>
> diff --git a/drivers/gpu/drm/radeon/radeon_mode.h
> b/drivers/gpu/drm/radeon/radeon_mode.h
> index fd470d6bf3f4..06c4c527d376 100644
> --- a/drivers/gpu/drm/radeon/radeon_mode.h
> +++ b/drivers/gpu/drm/radeon/radeon_mode.h
> @@ -881,6 +881,12 @@ extern int radeon_get_crtc_scanoutpos(struct drm_device
> *dev, unsigned int pipe,
> ktime_t *stime, ktime_t *etime,
> const struct drm_display_mode *mode);
>
> +extern bool radeon_get_crtc_scanout_position(struct drm_crtc *crtc,
> +bool in_vblank_irq, int *vpos,
> +int *hpos, ktime_t *stime,
> +
Re: [PATCH 02/23] drm/amdgpu: Convert to struct drm_crtc_helper_funcs.get_scanout_position()
On Fri, Jan 10, 2020 at 4:21 AM Thomas Zimmermann wrote:
>
> The callback struct drm_driver.get_scanout_position() is deprecated in
> favor of struct drm_crtc_helper_funcs.get_scanout_position(). Convert
> amdgpu over.
>
I would prefer to just change the signature of
amdgpu_display_get_crtc_scanoutpos() to match the new API rather than
wrapping it again.
Alex
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 12
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 11 ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 5 +
> drivers/gpu/drm/amd/amdgpu/dce_v10_0.c| 1 +
> drivers/gpu/drm/amd/amdgpu/dce_v11_0.c| 1 +
> drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 1 +
> drivers/gpu/drm/amd/amdgpu/dce_v8_0.c | 1 +
> drivers/gpu/drm/amd/amdgpu/dce_virtual.c | 1 +
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 ++-
> 9 files changed, 24 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index 4e699071d144..a1e769d4417d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -914,3 +914,15 @@ int amdgpu_display_crtc_idx_to_irq_type(struct
> amdgpu_device *adev, int crtc)
> return AMDGPU_CRTC_IRQ_NONE;
> }
> }
> +
> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
> + bool in_vblank_irq, int *vpos,
> + int *hpos, ktime_t *stime, ktime_t *etime,
> + const struct drm_display_mode *mode)
> +{
> + struct drm_device *dev = crtc->dev;
> + unsigned int pipe = crtc->index;
> +
> + return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
> + stime, etime, mode);
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 3f6f14ce1511..0749285dd1c7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1367,16 +1367,6 @@ int amdgpu_file_to_fpriv(struct file *filp, struct
> amdgpu_fpriv **fpriv)
> return 0;
> }
>
> -static bool
> -amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe,
> -bool in_vblank_irq, int *vpos, int *hpos,
> -ktime_t *stime, ktime_t *etime,
> -const struct drm_display_mode *mode)
> -{
> - return amdgpu_display_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos,
> - stime, etime, mode);
> -}
> -
> static struct drm_driver kms_driver = {
> .driver_features =
> DRIVER_USE_AGP | DRIVER_ATOMIC |
> @@ -1391,7 +1381,6 @@ static struct drm_driver kms_driver = {
> .enable_vblank = amdgpu_enable_vblank_kms,
> .disable_vblank = amdgpu_disable_vblank_kms,
> .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
> - .get_scanout_position = amdgpu_get_crtc_scanout_position,
> .irq_handler = amdgpu_irq_handler,
> .ioctls = amdgpu_ioctls_kms,
> .gem_free_object_unlocked = amdgpu_gem_object_free,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index eb9975f4decb..37ba07e2feb5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -612,6 +612,11 @@ void amdgpu_panel_mode_fixup(struct drm_encoder *encoder,
> struct drm_display_mode *adjusted_mode);
> int amdgpu_display_crtc_idx_to_irq_type(struct amdgpu_device *adev, int
> crtc);
>
> +bool amdgpu_crtc_get_scanout_position(struct drm_crtc *crtc,
> + bool in_vblank_irq, int *vpos,
> + int *hpos, ktime_t *stime, ktime_t *etime,
> + const struct drm_display_mode *mode);
> +
> /* fbdev layer */
> int amdgpu_fbdev_init(struct amdgpu_device *adev);
> void amdgpu_fbdev_fini(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 40d2ac723dd6..bdc1e0f036d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2685,6 +2685,7 @@ static const struct drm_crtc_helper_funcs
> dce_v10_0_crtc_helper_funcs = {
> .prepare = dce_v10_0_crtc_prepare,
> .commit = dce_v10_0_crtc_commit,
> .disable = dce_v10_0_crtc_disable,
> + .get_scanout_position = amdgpu_crtc_get_scanout_position,
> };
>
> static int dce_v10_0_crtc_init(struct amdgpu_device *adev, int index)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 898ef72d423c..0319da5f7
Re: [Intel-gfx] [PATCH 0/6]: drm: Introduce struct drm_device based WARN* and use them in i915
On Mon, Jan 13, 2020 at 02:33:36PM +0200, Jani Nikula wrote: > On Mon, 13 Jan 2020, Pankaj Bharadiya > wrote: > > Device specific dev_WARN and dev_WARN_ONCE macros available in kernel > > include device information in the backtrace, so we know what device > > the warnings originate from. > > > > Similar to this, add new struct drm_device based drm_WARN* macros. These > > macros include device information in the backtrace, so we know > > what device the warnings originate from. Knowing the device specific > > information in the backtrace would be helpful in development all > > around. > > > > This patch series aims to convert calls of WARN(), WARN_ON(), > > WARN_ONCE() and WARN_ON_ONCE() in i916 driver to use the drm > > device-specific variants automatically wherever struct device pointer > > is available. > > > > To do this, this patch series - > > - introduces new struct drm_device based WARN* macros > > - automatically converts WARN* with device specific dev_WARN* > > variants using coccinelle semantic patch scripts. > > > > The goal is to convert all the calls of WARN* with drm_WARN* in i915, > > but there are still cases where device pointer is not readily > > available in some functions (or I missed them somehow) using WARN* > > hence some manual churning is needed. Handle such remaining cases > > separately later. > > > > Changes since RFC at [1] > > - Introduce drm_WARN* macros and use them as suggested by Sam and Jani > > - Get rid of extra local variables > > > > [1] https://patchwork.freedesktop.org/series/71668/ > > > > Pankaj Bharadiya (6): > > drm/print: introduce new struct drm_device based WARN* macros > > drm/i915: add helper functions to get device ptr > > drm/i915: Make WARN* drm specific where drm_device ptr available > > drm/i915: Make WARN* drm specific where drm_priv ptr is available > > drm/i915: Make WARN* drm specific where dev_priv can be extracted. > > drm/i915: Make WARN* drm specific for various cases. > > I think the last four patches actually making the conversions are too > big, and need to be split out to smaller pieces. These are bound to > conflict in annoying ways. > > The absolute minimum I think it splitting by directory, and in > particular the changes to gvt needs to be handled by gvt maintainers. OK. Will split. Thanks, Pankaj > > BR, > Jani. > > > > > > drivers/gpu/drm/i915/display/icl_dsi.c| 14 +- > > drivers/gpu/drm/i915/display/intel_atomic.c | 6 +- > > drivers/gpu/drm/i915/display/intel_audio.c| 19 +- > > drivers/gpu/drm/i915/display/intel_bios.c | 10 +- > > drivers/gpu/drm/i915/display/intel_bw.c | 3 +- > > drivers/gpu/drm/i915/display/intel_cdclk.c| 81 +++--- > > drivers/gpu/drm/i915/display/intel_color.c| 3 +- > > .../gpu/drm/i915/display/intel_combo_phy.c| 2 +- > > .../gpu/drm/i915/display/intel_connector.c| 3 +- > > drivers/gpu/drm/i915/display/intel_crt.c | 10 +- > > drivers/gpu/drm/i915/display/intel_ddi.c | 100 > > drivers/gpu/drm/i915/display/intel_display.c | 239 ++ > > .../drm/i915/display/intel_display_power.c| 170 +++-- > > .../drm/i915/display/intel_display_types.h| 14 + > > drivers/gpu/drm/i915/display/intel_dp.c | 121 + > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 +- > > drivers/gpu/drm/i915/display/intel_dpio_phy.c | 3 +- > > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 39 +-- > > drivers/gpu/drm/i915/display/intel_dsb.c | 6 +- > > .../i915/display/intel_dsi_dcs_backlight.c| 2 +- > > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 5 +- > > drivers/gpu/drm/i915/display/intel_fbc.c | 23 +- > > drivers/gpu/drm/i915/display/intel_fbdev.c| 13 +- > > drivers/gpu/drm/i915/display/intel_gmbus.c| 3 +- > > drivers/gpu/drm/i915/display/intel_hdcp.c | 21 +- > > drivers/gpu/drm/i915/display/intel_hdmi.c | 71 +++--- > > drivers/gpu/drm/i915/display/intel_hotplug.c | 7 +- > > .../gpu/drm/i915/display/intel_lpe_audio.c| 2 +- > > drivers/gpu/drm/i915/display/intel_lvds.c | 7 +- > > drivers/gpu/drm/i915/display/intel_opregion.c | 7 +- > > drivers/gpu/drm/i915/display/intel_overlay.c | 18 +- > > drivers/gpu/drm/i915/display/intel_panel.c| 21 +- > > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 7 +- > > drivers/gpu/drm/i915/display/intel_psr.c | 32 ++- > > drivers/gpu/drm/i915/display/intel_sdvo.c | 17 +- > > drivers/gpu/drm/i915/display/intel_sprite.c | 5 +- > > drivers/gpu/drm/i915/display/intel_tc.c | 17 +- > > drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +- > > drivers/gpu/drm/i915/display/vlv_dsi.c| 2 +- > > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 4 +- > > drivers/gpu/drm/i915/gem/i915_gem_pm.c| 4 +- > > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 3 +- > > drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 13 +- > >
Re: [Intel-gfx] [PATCH 2/6] drm/i915: add helper functions to get device ptr
Hi Jani,
On Mon, Jan 13, 2020 at 02:14:51PM +0200, Jani Nikula wrote:
> On Mon, 13 Jan 2020, Pankaj Bharadiya
> wrote:
> > We will need struct drm_device pointer to pass it to drm_WARN* calls.
> >
> > Add helper functions to exract drm_device pointer from various structs.
>
> Please don't do this.
>
> We use the helpers we currently have when they involve something more
> complex than dereferences, such as container_of() or having to use
> dev_get_drvdata() or pci_get_drvdata().
Removing all the helpers will lead to 3 level access depth wherever
struct drm_device pointer is needed and unnecessary complicate the
drm_WARN* calls IMHO.
e.g.: drm_WARN(&vgpu->gvt->dev_priv.drm, 1)
If you are OK with this, I will remove all the helpers and extract
drm_device pointer while calling drm_WARN*() instead and submit
updated patch series.
Thanks,
Pankaj
>
> I really don't want people to use e.g.
>
> i915_to_dev(i915)
>
> over
>
> &i915->drm
>
>
> BR,
> Jani.
>
> >
> > Signed-off-by: Pankaj Bharadiya
> > ---
> > drivers/gpu/drm/i915/display/intel_display_types.h | 14 ++
> > drivers/gpu/drm/i915/gvt/gvt.h | 5 +
> > drivers/gpu/drm/i915/i915_drv.h| 11 +++
> > 3 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 7964b3dc0046..765878718fe9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1521,6 +1521,20 @@ dp_to_i915(struct intel_dp *intel_dp)
> > return to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> > }
> >
> > +static inline struct drm_device *enc_to_dev(const struct intel_encoder
> > *encoder)
> > +{
> > + return encoder->base.dev;
> > +}
> > +
> > +static inline struct drm_device *
> > +crtc_state_to_dev(const struct intel_crtc_state *state)
> > +{
> > + struct intel_crtc *crtc = to_intel_crtc(state->uapi.crtc);
> > + struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > +
> > + return i915_to_dev(i915);
> > +}
> > +
> > static inline struct intel_digital_port *
> > hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
> > {
> > diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> > index b47c6acaf9c0..d257399b075a 100644
> > --- a/drivers/gpu/drm/i915/gvt/gvt.h
> > +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> > @@ -348,6 +348,11 @@ static inline struct intel_gvt *to_gvt(struct
> > drm_i915_private *i915)
> > return i915->gvt;
> > }
> >
> > +static inline struct drm_device *vgpu_to_dev(const struct intel_vgpu *vgpu)
> > +{
> > + return i915_to_dev(vgpu->gvt->dev_priv);
> > +}
> > +
> > enum {
> > INTEL_GVT_REQUEST_EMULATE_VBLANK = 0,
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index e7be4c3e43c6..bdc89d021ff8 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1325,6 +1325,17 @@ static inline struct drm_i915_private
> > *pdev_to_i915(struct pci_dev *pdev)
> > return pci_get_drvdata(pdev);
> > }
> >
> > +static inline struct drm_device *i915_to_dev(struct drm_i915_private *i915)
> > +{
> > + return &i915->drm;
> > +}
> > +
> > +static inline struct drm_device *
> > +perf_stream_to_dev(const struct i915_perf_stream *stream)
> > +{
> > + return i915_to_dev(stream->perf->i915);
> > +}
> > +
> > /* Simple iterator over all initialised engines */
> > #define for_each_engine(engine__, dev_priv__, id__) \
> > for ((id__) = 0; \
>
> --
> Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/dp_mst: Have DP_Tx send one msg at a time
Hey! Haven't taken a look at this patch yet but just wanted to let you know
I'm going to try to get to most of the stuff you've got pending for me. I came
down with a really nasty cold last week so sorry if you've had any other
patches waiting until now!
On Mon, 2020-01-13 at 17:36 +0800, Wayne Lin wrote:
> [Why]
> Noticed this while testing MST with the 4 ports MST hub from
> StarTech.com. Sometimes can't light up monitors normally and get the
> error message as 'sideband msg build failed'.
>
> Look into aux transactions, found out that source sometimes will send
> out another down request before receiving the down reply of the
> previous down request. On the other hand, in drm_dp_get_one_sb_msg(),
> current code doesn't handle the interleaved replies case. Hence, source
> can't build up message completely and can't light up monitors.
>
> [How]
> For good compatibility, enforce source to send out one down request at a
> time. Add a flag, is_waiting_for_dwn_reply, to determine if the source
> can send out a down request immediately or not.
>
> - Check the flag before calling process_single_down_tx_qlock to send out
> a msg
> - Set the flag when successfully send out a down request
> - Clear the flag when successfully build up a down reply
> - Clear the flag when find erros during sending out a down request
> - Clear the flag when find errors during building up a down reply
> - Clear the flag when timeout occurs during waiting for a down reply
> - Use drm_dp_mst_kick_tx() to try to send another down request in queue
> at the end of drm_dp_mst_wait_tx_reply() (attempt to send out messages
> in queue when errors occur)
>
> Cc: Lyude Paul
> Signed-off-by: Wayne Lin
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 14 --
> include/drm/drm_dp_mst_helper.h | 6 ++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 4395d5cc0645..3542af15387a 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1195,6 +1195,8 @@ static int drm_dp_mst_wait_tx_reply(struct
> drm_dp_mst_branch *mstb,
> txmsg->state == DRM_DP_SIDEBAND_TX_SENT) {
> mstb->tx_slots[txmsg->seqno] = NULL;
> }
> + mgr->is_waiting_for_dwn_reply = false;
> +
> }
> out:
> if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
> @@ -1204,6 +1206,7 @@ static int drm_dp_mst_wait_tx_reply(struct
> drm_dp_mst_branch *mstb,
> }
> mutex_unlock(&mgr->qlock);
>
> + drm_dp_mst_kick_tx(mgr);
> return ret;
> }
>
> @@ -2770,9 +2773,11 @@ static void process_single_down_tx_qlock(struct
> drm_dp_mst_topology_mgr *mgr)
> ret = process_single_tx_qlock(mgr, txmsg, false);
> if (ret == 1) {
> /* txmsg is sent it should be in the slots now */
> + mgr->is_waiting_for_dwn_reply = true;
> list_del(&txmsg->next);
> } else if (ret) {
> DRM_DEBUG_KMS("failed to send msg in q %d\n", ret);
> + mgr->is_waiting_for_dwn_reply = false;
> list_del(&txmsg->next);
> if (txmsg->seqno != -1)
> txmsg->dst->tx_slots[txmsg->seqno] = NULL;
> @@ -2812,7 +2817,8 @@ static void drm_dp_queue_down_tx(struct
> drm_dp_mst_topology_mgr *mgr,
> drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
> }
>
> - if (list_is_singular(&mgr->tx_msg_downq))
> + if (list_is_singular(&mgr->tx_msg_downq) &&
> + !mgr->is_waiting_for_dwn_reply)
> process_single_down_tx_qlock(mgr);
> mutex_unlock(&mgr->qlock);
> }
> @@ -3743,6 +3749,7 @@ static int drm_dp_mst_handle_down_rep(struct
> drm_dp_mst_topology_mgr *mgr)
> mutex_lock(&mgr->qlock);
> txmsg->state = DRM_DP_SIDEBAND_TX_RX;
> mstb->tx_slots[slot] = NULL;
> + mgr->is_waiting_for_dwn_reply = false;
> mutex_unlock(&mgr->qlock);
>
> wake_up_all(&mgr->tx_waitq);
> @@ -3752,6 +3759,9 @@ static int drm_dp_mst_handle_down_rep(struct
> drm_dp_mst_topology_mgr *mgr)
> no_msg:
> drm_dp_mst_topology_put_mstb(mstb);
> clear_down_rep_recv:
> + mutex_lock(&mgr->qlock);
> + mgr->is_waiting_for_dwn_reply = false;
> + mutex_unlock(&mgr->qlock);
> memset(&mgr->down_rep_recv, 0, sizeof(struct drm_dp_sideband_msg_rx));
>
> return 0;
> @@ -4591,7 +4601,7 @@ static void drm_dp_tx_work(struct work_struct *work)
> struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
> drm_dp_mst_topology_mgr, tx_work);
>
> mutex_lock(&mgr->qlock);
> - if (!list_empty(&mgr->tx_msg_downq))
> + if (!list_empty(&mgr->tx_msg_downq) && !mgr->is_waiting_for_dwn_reply)
> process_single_down_tx_qlock(mgr);
> mutex_unlock(&mgr->qlock);
> }
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> i
Re: [Freedreno] [PATCH 1/2] drm/msm: Add a GPU-wide wait queue
On Mon, Jan 13, 2020 at 10:36:04AM -0500, Brian Ho wrote:
> This wait queue is signaled on all IRQs for a given GPU and will be
> used as part of the new MSM_WAIT_IOVA ioctl so userspace can sleep
> until the value at a given iova reaches a certain condition.
>
> Signed-off-by: Brian Ho
> ---
> drivers/gpu/drm/msm/msm_gpu.c | 4
> drivers/gpu/drm/msm/msm_gpu.h | 3 +++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
> index a052364a5d74..d7310c1336e5 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.c
> +++ b/drivers/gpu/drm/msm/msm_gpu.c
> @@ -779,6 +779,8 @@ void msm_gpu_submit(struct msm_gpu *gpu, struct
> msm_gem_submit *submit,
> static irqreturn_t irq_handler(int irq, void *data)
> {
> struct msm_gpu *gpu = data;
> + wake_up_all(&gpu->event);
> +
I suppose it is intentional to have this happen on *all* interrupts because you
might be using the CP interrupts for fun and profit and you don't want to plumb
in callbacks? I suppose it is okay to do this for all interrupts (including
errors) but if we're spending a lot of time here we might want to only trigger
on certain IRQs.
> return gpu->funcs->irq(gpu);
> }
>
> @@ -871,6 +873,8 @@ int msm_gpu_init(struct drm_device *drm, struct
> platform_device *pdev,
>
> spin_lock_init(&gpu->perf_lock);
>
> + init_waitqueue_head(&gpu->event);
> +
>
> /* Map registers: */
> gpu->mmio = msm_ioremap(pdev, config->ioname, name);
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index ab8f0f9c9dc8..60562f065dbc 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -104,6 +104,9 @@ struct msm_gpu {
>
> struct msm_gem_address_space *aspace;
>
> + /* GPU-wide wait queue that is signaled on all IRQs */
> + wait_queue_head_t event;
> +
> /* Power Control: */
> struct regulator *gpu_reg, *gpu_cx;
> struct clk_bulk_data *grp_clks;
> --
> 2.25.0.rc1.283.g88dfdc4193-goog
>
> ___
> Freedreno mailing list
> freedr...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 2/2] drm/msm: Add MSM_WAIT_IOVA ioctl
On Mon, Jan 13, 2020 at 10:36:05AM -0500, Brian Ho wrote:
> Implements an ioctl to wait until a value at a given iova is greater
> than or equal to a supplied value.
>
> This will initially be used by turnip (open-source Vulkan driver for
> QC in mesa) for occlusion queries where the userspace driver can
> block on a query becoming available before continuing via
> vkGetQueryPoolResults.
>
> Signed-off-by: Brian Ho
> ---
> drivers/gpu/drm/msm/msm_drv.c | 63 +--
> include/uapi/drm/msm_drm.h| 13
> 2 files changed, 74 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index c84f0a8b3f2c..dcc46874a5a2 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -36,10 +36,11 @@
> * MSM_GEM_INFO ioctl.
> * - 1.4.0 - softpin, MSM_RELOC_BO_DUMP, and GEM_INFO support to set/get
> * GEM object's debug name
> - * - 1.5.0 - Add SUBMITQUERY_QUERY ioctl
> + * - 1.5.0 - Add SUBMITQUEUE_QUERY ioctl
> + * - 1.6.0 - Add WAIT_IOVA ioctl
> */
> #define MSM_VERSION_MAJOR1
> -#define MSM_VERSION_MINOR5
> +#define MSM_VERSION_MINOR6
> #define MSM_VERSION_PATCHLEVEL 0
>
> static const struct drm_mode_config_funcs mode_config_funcs = {
> @@ -952,6 +953,63 @@ static int msm_ioctl_submitqueue_close(struct drm_device
> *dev, void *data,
> return msm_submitqueue_remove(file->driver_priv, id);
> }
>
> +static int msm_ioctl_wait_iova(struct drm_device *dev, void *data,
> + struct drm_file *file)
> +{
> + struct msm_drm_private *priv = dev->dev_private;
> + struct drm_gem_object *obj;
> + struct drm_msm_wait_iova *args = data;
> + ktime_t timeout = to_ktime(args->timeout);
> + unsigned long remaining_jiffies = timeout_to_jiffies(&timeout);
> + struct msm_gpu *gpu = priv->gpu;
> + void *base_vaddr;
> + uint64_t *vaddr;
> + int ret;
> +
> + if (args->pad)
> + return -EINVAL;
> +
> + if (!gpu)
> + return 0;
If the GPU isn't up, it should be an error since this macro is specifically
designed for just the GPU (though, I suppose the display *COULD* use it to watch
a memory mapped register or something).
> +
> + obj = drm_gem_object_lookup(file, args->handle);
> + if (!obj)
> + return -ENOENT;
> +
> + base_vaddr = msm_gem_get_vaddr(obj);
> + if (IS_ERR(base_vaddr)) {
> + ret = PTR_ERR(base_vaddr);
> + goto err_put_gem_object;
> + }
> + if (args->offset + sizeof(*vaddr) > obj->size) {
There is a chance to trigger a u64 overflow here resulting in an arbitrary (ish)
vaddr two lines below.
> + ret = -EINVAL;
> + goto err_put_vaddr;
> + }
You can check this before getting the vaddr which would save you a clean up
step.
> +
> + vaddr = base_vaddr + args->offset;
> +
> + /* Assumes WC mapping */
> + ret = wait_event_interruptible_timeout(
> + gpu->event, *vaddr >= args->value, remaining_jiffies);
I feel like a barrier might be needed before checking *vaddr just in case you
get the interrupt and wake up the queue before the write posts from the
hardware.
> +
> + if (ret == 0) {
> + ret = -ETIMEDOUT;
> + goto err_put_vaddr;
> + } else if (ret == -ERESTARTSYS) {
> + goto err_put_vaddr;
> + }
You don't need either goto here because both paths execute the following cleanup
steps. I'm also not sure you need to worry about explicitly checking the
ERESTARTSYS value, I think that this would be sufficient:
if (ret == 0)
ret = -ETIMEDOUT;
else if (ret > 0)
ret = 0;
> +
Put your err_put_vaddr: label here, but looking up, if you move the bounds check
before the msm_gem_get_vaddr, I don't think you need a label.
> + msm_gem_put_vaddr(obj);
Put the err_put_gem_object: label here.
> + drm_gem_object_put_unlocked(obj);
> + return 0;
return ret;
> +
> +err_put_vaddr:
> + msm_gem_put_vaddr(obj);
> +err_put_gem_object:
> + drm_gem_object_put_unlocked(obj);
> + return ret;
> +}
And then these guys aren't needed.
> +
> static const struct drm_ioctl_desc msm_ioctls[] = {
> DRM_IOCTL_DEF_DRV(MSM_GET_PARAM,msm_ioctl_get_param,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(MSM_GEM_NEW, msm_ioctl_gem_new,
> DRM_RENDER_ALLOW),
> @@ -964,6 +1022,7 @@ static const struct drm_ioctl_desc msm_ioctls[] = {
> DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_NEW, msm_ioctl_submitqueue_new,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_CLOSE, msm_ioctl_submitqueue_close,
> DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(MSM_SUBMITQUEUE_QUERY, msm_ioctl_submitqueue_query,
> DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(MSM_WAIT_IOVA, msm_ioctl_wait_iova, DRM_RENDER_ALLOW),
> };
>
> static const struct vm_operations_struct vm_ops = {
> diff --git a/inc
Re: [PATCH v2 2/4] drm/msm: allow zapfw to not be specified in gpulist
On Sun, Jan 12, 2020 at 11:53:58AM -0800, Rob Clark wrote:
> From: Rob Clark
>
> For newer devices we want to require the path to come from the
> firmware-name property in the zap-shader dt node.
Reviewed-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 22 +-
> 1 file changed, 13 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 456bb5af1717..c146c3b8f52b 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -79,9 +79,21 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
> char *fwname,
> ret = request_firmware_direct(&fw, fwname, gpu->dev->dev);
> if (ret)
> fw = ERR_PTR(ret);
> - } else {
> + } else if (fwname) {
> /* Request the MDT file from the default location: */
> fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> + } else {
> + /*
> + * For new targets, we require the firmware-name property,
> + * if a zap-shader is required, rather than falling back
> + * to a firmware name specified in gpulist.
> + *
> + * Because the firmware is signed with a (potentially)
> + * device specific key, having the name come from gpulist
> + * was a bad idea, and is only provided for backwards
> + * compatibility for older targets.
> + */
> + return -ENODEV;
> }
A possible future optimization would be to move a lot of this to the target
specific code but we can do that once we're sure that all the rest of the
fallout has bee militated.
>
> if (IS_ERR(fw)) {
> @@ -170,14 +182,6 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32
> pasid)
> return -EPROBE_DEFER;
> }
>
> - /* Each GPU has a target specific zap shader firmware name to use */
> - if (!adreno_gpu->info->zapfw) {
> - zap_available = false;
> - DRM_DEV_ERROR(&pdev->dev,
> - "Zap shader firmware file not specified for this
> target\n");
> - return -ENODEV;
> - }
> -
> return zap_shader_load_mdt(gpu, adreno_gpu->info->zapfw, pasid);
> }
>
> --
> 2.24.1
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 3/3] drm/panel: simple: Add support for the Frida FRD350H54004 panel
Hi Paul.
On Mon, Jan 13, 2020 at 01:17:41PM -0300, Paul Cercueil wrote:
> The FRD350H54004 is a simple 3.5" 320x240 24-bit TFT panel, found for
> instance inside the Anbernic RG-350 handheld gaming console.
>
> v2: Order alphabetically
> v3: Add connector_type, and update timings according to the constraints
> listed in the datasheet
>
> Signed-off-by: Paul Cercueil
Thanks, applied to drm-misc-next.
Sam
> ---
> drivers/gpu/drm/panel/panel-simple.c | 30
> 1 file changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/panel/panel-simple.c
> b/drivers/gpu/drm/panel/panel-simple.c
> index e14c14ac62b5..d6f77bc494c7 100644
> --- a/drivers/gpu/drm/panel/panel-simple.c
> +++ b/drivers/gpu/drm/panel/panel-simple.c
> @@ -1440,6 +1440,33 @@ static const struct panel_desc foxlink_fl500wvr00_a0t
> = {
> .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> };
>
> +static const struct drm_display_mode frida_frd350h54004_mode = {
> + .clock = 6000,
> + .hdisplay = 320,
> + .hsync_start = 320 + 44,
> + .hsync_end = 320 + 44 + 16,
> + .htotal = 320 + 44 + 16 + 20,
> + .vdisplay = 240,
> + .vsync_start = 240 + 2,
> + .vsync_end = 240 + 2 + 6,
> + .vtotal = 240 + 2 + 6 + 2,
> + .vrefresh = 60,
> + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC,
> +};
> +
> +static const struct panel_desc frida_frd350h54004 = {
> + .modes = &frida_frd350h54004_mode,
> + .num_modes = 1,
> + .bpc = 8,
> + .size = {
> + .width = 77,
> + .height = 64,
> + },
> + .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
> + .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE,
> + .connector_type = DRM_MODE_CONNECTOR_DPI,
> +};
> +
> static const struct drm_display_mode friendlyarm_hd702e_mode = {
> .clock = 67185,
> .hdisplay = 800,
> @@ -3309,6 +3336,9 @@ static const struct of_device_id platform_of_match[] = {
> }, {
> .compatible = "foxlink,fl500wvr00-a0t",
> .data = &foxlink_fl500wvr00_a0t,
> + }, {
> + .compatible = "frida,frd350h54004",
> + .data = &frida_frd350h54004,
> }, {
> .compatible = "friendlyarm,hd702e",
> .data = &friendlyarm_hd702e,
> --
> 2.24.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/3] dt-bindings: panel-simple: Add compatible for Frida FRD350H54004 LCD
Hi Paul. On Mon, Jan 13, 2020 at 01:17:40PM -0300, Paul Cercueil wrote: > Add bindings documentation for the Frida 3.5" (320x240 pixels) 24-bit > TFT LCD panel. > > v2: Switch documentation from plain text to YAML > v3: Simply add new compatible to panel-simple.yaml file instead of > adding new file > > Signed-off-by: Paul Cercueil Thanks, applied to drm-misc-next. Sam > --- > .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > index 8fe60ee2531c..4a8064e31793 100644 > --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml > @@ -37,6 +37,8 @@ properties: >- auo,b116xa01 > # BOE NV140FHM-N49 14.0" FHD a-Si FT panel >- boe,nv140fhmn49 > +# Frida FRD350H54004 3.5" QVGA TFT LCD panel > + - frida,frd350h54004 > # GiantPlus GPM940B0 3.0" QVGA TFT LCD panel >- giantplus,gpm940b0 > # Satoz SAT050AT40H12R2 5.0" WVGA TFT LCD panel > -- > 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/3] dt-bindings: vendor-prefixes: Add Shenzhen Frida LCD Co., Ltd.
Hi Paul. On Mon, Jan 13, 2020 at 01:17:39PM -0300, Paul Cercueil wrote: > Add an entry for Shenzhen Frida LCD Co., Ltd. > > v2: No change > v3: No change > > Signed-off-by: Paul Cercueil > Acked-by: Sam Ravnborg > Acked-by: Rob Herring Applied to drm-misc-next. Sam > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml > b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 835579edc971..76069bb51ea4 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -337,6 +337,8 @@ patternProperties: > description: Firefly >"^focaltech,.*": > description: FocalTech Systems Co.,Ltd > + "^frida,.*": > +description: Shenzhen Frida LCD Co., Ltd. >"^friendlyarm,.*": > description: Guangzhou FriendlyARM Computer Tech Co., Ltd >"^fsl,.*": > -- > 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/4] drm/msm: support firmware-name for zap fw (v2)
On Sun, Jan 12, 2020 at 11:53:57AM -0800, Rob Clark wrote:
> From: Rob Clark
>
> Since zap firmware can be device specific, allow for a firmware-name
> property in the zap node to specify which firmware to load, similarly to
> the scheme used for dsp/wifi/etc.
>
> v2: only need a single error msg when we can't load from firmware-name
> specified path, and fix comment [Bjorn A.]
Reviewed-by: Jordan Crouse
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 30 ++---
> 1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 112e8b8a261e..456bb5af1717 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
> char *fwname,
> {
> struct device *dev = &gpu->pdev->dev;
> const struct firmware *fw;
> + const char *signed_fwname = NULL;
> struct device_node *np, *mem_np;
> struct resource r;
> phys_addr_t mem_phys;
> @@ -58,8 +59,31 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
> char *fwname,
>
> mem_phys = r.start;
>
> - /* Request the MDT file for the firmware */
> - fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> + /*
> + * Check for a firmware-name property. This is the new scheme
> + * to handle firmware that may be signed with device specific
> + * keys, allowing us to have a different zap fw path for different
> + * devices.
> + *
> + * If the firmware-name property is found, we bypass the
> + * adreno_request_fw() mechanism, because we don't need to handle
> + * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> + *
> + * If the firmware-name property is not found, for backwards
> + * compatibility we fall back to the fwname from the gpulist
> + * table.
> + */
> + of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> + if (signed_fwname) {
> + fwname = signed_fwname;
> + ret = request_firmware_direct(&fw, fwname, gpu->dev->dev);
> + if (ret)
> + fw = ERR_PTR(ret);
> + } else {
> + /* Request the MDT file from the default location: */
> + fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> + }
> +
> if (IS_ERR(fw)) {
> DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
> return PTR_ERR(fw);
> @@ -95,7 +119,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const
> char *fwname,
>* not. But since we've already gotten through adreno_request_fw()
>* we know which of the two cases it is:
>*/
> - if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
> + if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY))
> {
> ret = qcom_mdt_load(dev, fw, fwname, pasid,
> mem_region, mem_phys, mem_size, NULL);
> } else {
> --
> 2.24.1
>
--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm: Add getfb2 ioctl
On Mon, Dec 16, 2019 at 07:46:43PM -0800, Juston Li wrote:
> From: Daniel Stone
>
> getfb2 allows us to pass multiple planes and modifiers, just like addfb2
> over addfb.
>
> Changes since v2:
> - add privilege checks from getfb1 since handles should only be
>returned to master/root
>
> Changes since v1:
> - unused modifiers set to 0 instead of DRM_FORMAT_MOD_INVALID
> - update ioctl number
>
> Signed-off-by: Daniel Stone
> Signed-off-by: Juston Li
> Acked-by: Daniel Vetter
lgtm
Reviewed-by: Ville Syrjälä
> ---
> drivers/gpu/drm/drm_crtc_internal.h | 2 +
> drivers/gpu/drm/drm_framebuffer.c | 122
> drivers/gpu/drm/drm_ioctl.c | 1 +
> include/uapi/drm/drm.h | 2 +
> 4 files changed, 127 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h
> b/drivers/gpu/drm/drm_crtc_internal.h
> index c7d5e4c21423..16f2413403aa 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -216,6 +216,8 @@ int drm_mode_rmfb_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
> int drm_mode_getfb(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
> +int drm_mode_getfb2_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv);
> int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv);
>
> diff --git a/drivers/gpu/drm/drm_framebuffer.c
> b/drivers/gpu/drm/drm_framebuffer.c
> index 57564318ceea..57ac94ce9b9e 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -548,7 +549,128 @@ int drm_mode_getfb(struct drm_device *dev,
>
> out:
> drm_framebuffer_put(fb);
> + return ret;
> +}
> +
> +/**
> + * drm_mode_getfb2 - get extended FB info
> + * @dev: drm device for the ioctl
> + * @data: data pointer for the ioctl
> + * @file_priv: drm file for the ioctl call
> + *
> + * Lookup the FB given its ID and return info about it.
> + *
> + * Called by the user via ioctl.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_mode_getfb2_ioctl(struct drm_device *dev,
> + void *data, struct drm_file *file_priv)
> +{
> + struct drm_mode_fb_cmd2 *r = data;
> + struct drm_framebuffer *fb;
> + unsigned int i;
> + int ret;
> +
> + if (!drm_core_check_feature(dev, DRIVER_MODESET))
> + return -EINVAL;
> +
> + fb = drm_framebuffer_lookup(dev, file_priv, r->fb_id);
> + if (!fb)
> + return -ENOENT;
> +
> + /* For multi-plane framebuffers, we require the driver to place the
> + * GEM objects directly in the drm_framebuffer. For single-plane
> + * framebuffers, we can fall back to create_handle.
> + */
> + if (!fb->obj[0] &&
> + (fb->format->num_planes > 1 || !fb->funcs->create_handle)) {
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + r->height = fb->height;
> + r->width = fb->width;
> + r->pixel_format = fb->format->format;
> +
> + r->flags = 0;
> + if (dev->mode_config.allow_fb_modifiers)
> + r->flags |= DRM_MODE_FB_MODIFIERS;
> +
> + for (i = 0; i < ARRAY_SIZE(r->handles); i++) {
> + r->handles[i] = 0;
> + r->pitches[i] = 0;
> + r->offsets[i] = 0;
> + r->modifier[i] = 0;
> + }
> +
> + for (i = 0; i < fb->format->num_planes; i++) {
> + r->pitches[i] = fb->pitches[i];
> + r->offsets[i] = fb->offsets[i];
> + if (dev->mode_config.allow_fb_modifiers)
> + r->modifier[i] = fb->modifier;
> + }
> +
> + /* GET_FB2() is an unprivileged ioctl so we must not return a
> + * buffer-handle to non master/root processes! To match GET_FB()
> + * just return invalid handles (0) for non masters/root
> + * rather than making GET_FB2() privileged.
> + */
> + if (!drm_is_current_master(file_priv) && !capable(CAP_SYS_ADMIN)) {
> + ret = 0;
> + goto out;
> + }
>
> + for (i = 0; i < fb->format->num_planes; i++) {
> + int j;
> +
> + /* If we reuse the same object for multiple planes, also
> + * return the same handle.
> + */
> + for (j = 0; j < i; j++) {
> + if (fb->obj[i] == fb->obj[j]) {
> + r->handles[i] = r->handles[j];
> + break;
> + }
> + }
> +
> + if (r->handles[i])
> + continue;
> +
> + if (fb->obj[i]) {
> + ret = drm_gem_handle_create(file_priv, fb->obj[i],
> +
Re: [PATCH RFT v1 3/3] drm/panfrost: Use the mali-supply regulator for control again
On 09/01/2020 17:27, Martin Blumenstingl wrote:
> On Thu, Jan 9, 2020 at 12:31 PM Steven Price wrote:
>>
>> On 07/01/2020 23:06, Martin Blumenstingl wrote:
>>> dev_pm_opp_set_rate() needs a reference to the regulator which should be
>>> updated when updating the GPU frequency. The name of the regulator has
>>> to be passed at initialization-time using dev_pm_opp_set_regulators().
>>> Add the call to dev_pm_opp_set_regulators() so dev_pm_opp_set_rate()
>>> will update the GPU regulator when updating the frequency (just like
>>> we did this manually before when we open-coded dev_pm_opp_set_rate()).
>>
>> This patch causes a warning from debugfs on my firefly (RK3288) board:
>>
>> debugfs: Directory 'ffa3.gpu-mali' with parent 'vdd_gpu' already
>> present!
>>
>> So it looks like the regulator is being added twice - but I haven't
>> investigated further.
> I *think* it's because the regulator is already fetched by the
> panfrost driver itself to enable it
> (the devfreq code currently does not support enabling the regulator,
> it can only control the voltage)
>
> I'm not sure what to do about this though
Having a little play around with this, I think you can simply remove the
panfrost_regulator_init() call. This at least works for me - the call to
dev_pm_opp_set_regulators() seems to set everything up. However I
suspect you need to do this unconditionally even if there are no
operating points defined.
> [...]
>>> ret = dev_pm_opp_of_add_table(dev);
>>> - if (ret)
>>> + if (ret) {
>>> +
>>> dev_pm_opp_put_regulators(pfdev->devfreq.regulators_opp_table);
>>
>> If we don't have a regulator then regulators_opp_table will be NULL and
>> sadly dev_pm_opp_put_regulators() doesn't handle a NULL argument. The
>> same applies to the two below calls obviously.
> good catch, thank you!
> are you happy with the general approach here or do you think that
> dev_pm_opp_set_regulators is the wrong way to go (for whatever
> reason)?
To be honest this is an area I still don't fully understand. There's a
lot of magic helper functions and very little in the way of helpful
documentation to work out which are the right ones to call. It seems
reasonable to me, hopefully someone more in the know will chime in it
there's something fundamentally wrong!
Thanks,
Steve
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/display: Use u64 divide macro for round up division
On Mon, Jan 13, 2020 at 08:20:42AM -0500, mikita.lip...@amd.com wrote: > From: Mikita Lipski > > [why] > Fix compilation warnings on i386 architecture: > undefined reference to `__udivdi3' > [how] > Switch DIV_ROUND_UP to DIV64_U64_ROUND_UP > > Reported-by: Randy Dunlap > Signed-off-by: Mikita Lipski > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 52fb207393ef..96b391e4b3e7 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -534,7 +534,7 @@ static int kbps_to_peak_pbn(int kbps) > > peak_kbps *= 1006; > peak_kbps = div_u64(peak_kbps, 1000); > - return (int) DIV_ROUND_UP(peak_kbps * 64, (54 * 8 * 1000)); > + return (int) DIV64_U64_ROUND_UP(peak_kbps * 64, (54 * 8 * 1000)); DIV_ROUND_UP_ULL() would seem to suffice for this divisor. > } > > static void set_dsc_configs_from_fairness_vars(struct > dsc_mst_fairness_params *params, > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu/display: Use u64 divide macro for round up division
On 2020-01-13 8:20 a.m., mikita.lip...@amd.com wrote: > From: Mikita Lipski > > [why] > Fix compilation warnings on i386 architecture: > undefined reference to `__udivdi3' > [how] > Switch DIV_ROUND_UP to DIV64_U64_ROUND_UP > > Reported-by: Randy Dunlap > Signed-off-by: Mikita Lipski Reviewed-by: Harry Wentland Harry > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > index 52fb207393ef..96b391e4b3e7 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c > @@ -534,7 +534,7 @@ static int kbps_to_peak_pbn(int kbps) > > peak_kbps *= 1006; > peak_kbps = div_u64(peak_kbps, 1000); > - return (int) DIV_ROUND_UP(peak_kbps * 64, (54 * 8 * 1000)); > + return (int) DIV64_U64_ROUND_UP(peak_kbps * 64, (54 * 8 * 1000)); > } > > static void set_dsc_configs_from_fairness_vars(struct > dsc_mst_fairness_params *params, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206141] VCE UVD ring test failed -110
https://bugzilla.kernel.org/show_bug.cgi?id=206141 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) --- The relevant function is amdgpu_ib_ring_tests(), however, if the relevant engines are in some bad state, increasing the timeout won't help. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
DisplayLink UDL driver with Intel IOMMU enabled
I just came across a DMAR allocation issue when I wanted to use the UDL driver with a DL-165 chipset and had the Intel IOMMU enabled. It seems like it could be solved with the same patch already applied to the EVDI driver: https://github.com/DisplayLink/evdi/commit/74e289906dba95c0ad3208cfa6a57ba3042b930f So just add interface->dev.archdata.iommu = INTEL_IOMMU_DUMMY_DOMAIN; before the drm_dev_alloc call in udl_drv.c seems to be enough to use the UDL driver with IOMMU enabled. Kind regards, Volker ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/modes: Apply video parameters with only reflect option
Hi Maxime,
On Mon, Dec 16, 2019 at 07:08:12PM +0100, Stephan Gerhold wrote:
> On Mon, Dec 16, 2019 at 07:27:25PM +0200, Ville Syrjälä wrote:
> > On Mon, Dec 16, 2019 at 06:10:17PM +0100, Stephan Gerhold wrote:
> > > At the moment, video mode parameters like video=540x960,reflect_x,
> > > (without rotation set) are not taken into account when applying the
> > > mode in drm_client_modeset.c.
> >
> > A rotation value without exactly one rotation angle is illegal.
> > IMO the code should not generate a value like that in the first
> > place.
> >
What do you think about Ville's comment?
Should we change the command line parser to generate ROTATE_0 when no
rotate option is specified? This would also require updating a few of
the test cases.
You added the code for parsing the rotation/reflection options,
so I thought I'd ask before I start working on this.
Thanks,
Stephan
>
> You're right. I was thinking about this when creating this patch.
> But then I was not sure if "mode->rotation_reflection" is supposed to be
> a valid rotation value.The zero value is currently used to check
> if any rotation/reflection was specified at all:
>
> [...]
> > > diff --git a/drivers/gpu/drm/drm_client_modeset.c
> > > b/drivers/gpu/drm/drm_client_modeset.c
> > > index 895b73f23079..cfebce4f19a5 100644
> > > --- a/drivers/gpu/drm/drm_client_modeset.c
> > > +++ b/drivers/gpu/drm/drm_client_modeset.c
> > > @@ -859,19 +859,23 @@ bool drm_client_rotation(struct drm_mode_set
> > > *modeset, unsigned int *rotation)
> > >*/
> > > cmdline = &connector->cmdline_mode;
> > > if (cmdline->specified && cmdline->rotation_reflection) {
>
> I can try to make your suggested change in the cmdline parsing code,
> but since this sounds like a larger change I would appreciate
> some other opinions first.
>
> Thanks,
> Stephan
>
> > >
> > > - unsigned int cmdline_rest, panel_rest;
> > > - unsigned int cmdline_rot, panel_rot;
> > > - unsigned int sum_rot, sum_rest;
> > > + if (cmdline->rotation_reflection & DRM_MODE_ROTATE_MASK) {
> > > + unsigned int cmdline_rest, panel_rest;
> > > + unsigned int cmdline_rot, panel_rot;
> > > + unsigned int sum_rot, sum_rest;
> > >
> > > - panel_rot = ilog2(*rotation & DRM_MODE_ROTATE_MASK);
> > > - cmdline_rot = ilog2(cmdline->rotation_reflection &
> > > DRM_MODE_ROTATE_MASK);
> > > - sum_rot = (panel_rot + cmdline_rot) % 4;
> > > + panel_rot = ilog2(*rotation & DRM_MODE_ROTATE_MASK);
> > > + cmdline_rot = ilog2(cmdline->rotation_reflection &
> > > DRM_MODE_ROTATE_MASK);
> > > + sum_rot = (panel_rot + cmdline_rot) % 4;
> > >
> > > - panel_rest = *rotation & ~DRM_MODE_ROTATE_MASK;
> > > - cmdline_rest = cmdline->rotation_reflection &
> > > ~DRM_MODE_ROTATE_MASK;
> > > - sum_rest = panel_rest ^ cmdline_rest;
> > > + panel_rest = *rotation & ~DRM_MODE_ROTATE_MASK;
> > > + cmdline_rest = cmdline->rotation_reflection &
> > > ~DRM_MODE_ROTATE_MASK;
> > > + sum_rest = panel_rest ^ cmdline_rest;
> > >
> > > - *rotation = (1 << sum_rot) | sum_rest;
> > > + *rotation = (1 << sum_rot) | sum_rest;
> > > + } else {
> > > + *rotation ^= cmdline->rotation_reflection;
> > > + }
> > > }
> > >
> > > /*
> > > @@ -879,7 +883,8 @@ bool drm_client_rotation(struct drm_mode_set
> > > *modeset, unsigned int *rotation)
> > >* depending on the hardware this may require the framebuffer
> > >* to be in a specific tiling format.
> > >*/
> > > - if ((*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_180 ||
> > > + if (((*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_0 &&
> > > + (*rotation & DRM_MODE_ROTATE_MASK) != DRM_MODE_ROTATE_180) ||
> > > !plane->rotation_property)
> > > return false;
> > >
> > > --
> > > 2.24.1
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Ville Syrjälä
> > Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC 3/3] drm/ttm: support memcg for ttm_tt
Am 13.01.20 um 16:35 schrieb Qiang Yu:
Charge TTM allocated system memory to memory cgroup which will
limit the memory usage of a group of processes.
NAK to the whole approach. This belongs into the GEM or driver layer,
but not into TTM.
The memory is always charged to the control group of task which
create this buffer object and when it's created. For example,
when a buffer is created by process A and exported to process B,
then process B populate this buffer, the memory is still charged
to process A's memcg; if a buffer is created by process A when in
memcg B, then A is moved to memcg C and populate this buffer, it
will charge memcg B.
This is actually the most common use case for graphics application where
the X server allocates most of the backing store.
So we need a better handling than just accounting the memory to whoever
allocated it first.
Regards,
Christian.
Signed-off-by: Qiang Yu
---
drivers/gpu/drm/ttm/ttm_bo.c | 10 ++
drivers/gpu/drm/ttm/ttm_page_alloc.c | 18 +-
drivers/gpu/drm/ttm/ttm_tt.c | 3 +++
include/drm/ttm/ttm_bo_api.h | 5 +
include/drm/ttm/ttm_tt.h | 4
5 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 8d91b0428af1..4e64846ee523 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
static void ttm_bo_global_kobj_release(struct kobject *kobj);
@@ -162,6 +163,10 @@ static void ttm_bo_release_list(struct kref *list_kref)
if (!ttm_bo_uses_embedded_gem_object(bo))
dma_resv_fini(&bo->base._resv);
mutex_destroy(&bo->wu_mutex);
+#ifdef CONFIG_MEMCG
+ if (bo->memcg)
+ css_put(&bo->memcg->css);
+#endif
bo->destroy(bo);
ttm_mem_global_free(&ttm_mem_glob, acc_size);
}
@@ -1330,6 +1335,11 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
}
atomic_inc(&ttm_bo_glob.bo_count);
+#ifdef CONFIG_MEMCG
+ if (bo->type == ttm_bo_type_device)
+ bo->memcg = mem_cgroup_driver_get_from_current();
+#endif
+
/*
* For ttm_bo_type_device buffers, allocate
* address space from the device.
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b40a4678c296..ecd1831a1d38 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -42,7 +42,7 @@
#include /* for seq_printf */
#include
#include
-
+#include
#include
#include
@@ -1045,6 +1045,11 @@ ttm_pool_unpopulate_helper(struct ttm_tt *ttm, unsigned
mem_count_update)
ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
ttm->caching_state);
ttm->state = tt_unpopulated;
+
+#ifdef CONFIG_MEMCG
+ if (ttm->memcg)
+ mem_cgroup_uncharge_drvmem(ttm->memcg, ttm->num_pages);
+#endif
}
int ttm_pool_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
@@ -1059,6 +1064,17 @@ int ttm_pool_populate(struct ttm_tt *ttm, struct
ttm_operation_ctx *ctx)
if (ttm_check_under_lowerlimit(mem_glob, ttm->num_pages, ctx))
return -ENOMEM;
+#ifdef CONFIG_MEMCG
+ if (ttm->memcg) {
+ gfp_t gfp_flags = GFP_USER;
+ if (ttm->page_flags & TTM_PAGE_FLAG_NO_RETRY)
+ gfp_flags |= __GFP_RETRY_MAYFAIL;
+ ret = mem_cgroup_charge_drvmem(ttm->memcg, gfp_flags,
ttm->num_pages);
+ if (ret)
+ return ret;
+ }
+#endif
+
ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
ttm->caching_state);
if (unlikely(ret != 0)) {
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index e0e9b4f69db6..1acb153084e1 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -233,6 +233,9 @@ void ttm_tt_init_fields(struct ttm_tt *ttm, struct
ttm_buffer_object *bo,
ttm->state = tt_unpopulated;
ttm->swap_storage = NULL;
ttm->sg = bo->sg;
+#ifdef CONFIG_MEMCG
+ ttm->memcg = bo->memcg;
+#endif
}
int ttm_tt_init(struct ttm_tt *ttm, struct ttm_buffer_object *bo,
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 65e399d280f7..95a08e81a73e 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -54,6 +54,8 @@ struct ttm_place;
struct ttm_lru_bulk_move;
+struct mem_cgroup;
+
/**
* struct ttm_bus_placement
*
@@ -180,6 +182,9 @@ struct ttm_buffer_object {
void (*destroy) (struct ttm_buffer_object *);
unsigned long num_pages;
size_t acc_size;
+#ifdef CONFIG_MEMCG
+ struct mem_cgroup *memcg;
+#endif
/**
* Members not needing protection.
diff --git a/include/drm/ttm/ttm_t
Re: [PATCH] drm/ttm: nuke invalidate_caches callback
On Mon, Jan 13, 2020 at 10:45:25PM +0800, Christian König wrote:
> Ping? Just a trivial cleanup.
>
> Am 10.01.20 um 16:09 schrieb Christian König:
> > Another completely unused feature.
> >
> > Signed-off-by: Christian König
Reviewed-by: Huang Rui
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 --
> > drivers/gpu/drm/nouveau/nouveau_bo.c | 8
> > drivers/gpu/drm/qxl/qxl_ttm.c | 6 --
> > drivers/gpu/drm/radeon/radeon_ttm.c| 6 --
> > drivers/gpu/drm/ttm/ttm_bo.c | 9 +
> > drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 6 --
> > include/drm/ttm/ttm_bo_driver.h| 15 ---
> > 7 files changed, 1 insertion(+), 55 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 445de594c214..7c4b1cbd9a50 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -68,11 +68,6 @@ static int amdgpu_map_buffer(struct ttm_buffer_object
> > *bo,
> > static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev);
> > static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev);
> >
> > -static int amdgpu_invalidate_caches(struct ttm_bo_device *bdev, uint32_t
> > flags)
> > -{
> > - return 0;
> > -}
> > -
> > /**
> >* amdgpu_init_mem_type - Initialize a memory manager for a specific type
> > of
> >* memory request.
> > @@ -1637,7 +1632,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
> > .ttm_tt_create = &amdgpu_ttm_tt_create,
> > .ttm_tt_populate = &amdgpu_ttm_tt_populate,
> > .ttm_tt_unpopulate = &amdgpu_ttm_tt_unpopulate,
> > - .invalidate_caches = &amdgpu_invalidate_caches,
> > .init_mem_type = &amdgpu_init_mem_type,
> > .eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
> > .evict_flags = &amdgpu_evict_flags,
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > index f8015e0318d7..81668104595f 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > @@ -646,13 +646,6 @@ nouveau_ttm_tt_create(struct ttm_buffer_object *bo,
> > uint32_t page_flags)
> > return nouveau_sgdma_create_ttm(bo, page_flags);
> > }
> >
> > -static int
> > -nouveau_bo_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
> > -{
> > - /* We'll do this from user space. */
> > - return 0;
> > -}
> > -
> > static int
> > nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > struct ttm_mem_type_manager *man)
> > @@ -1696,7 +1689,6 @@ struct ttm_bo_driver nouveau_bo_driver = {
> > .ttm_tt_create = &nouveau_ttm_tt_create,
> > .ttm_tt_populate = &nouveau_ttm_tt_populate,
> > .ttm_tt_unpopulate = &nouveau_ttm_tt_unpopulate,
> > - .invalidate_caches = nouveau_bo_invalidate_caches,
> > .init_mem_type = nouveau_bo_init_mem_type,
> > .eviction_valuable = ttm_bo_eviction_valuable,
> > .evict_flags = nouveau_bo_evict_flags,
> > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> > index 16a5e903533d..62a5e424971b 100644
> > --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> > @@ -48,11 +48,6 @@ static struct qxl_device *qxl_get_qdev(struct
> > ttm_bo_device *bdev)
> > return qdev;
> > }
> >
> > -static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t
> > flags)
> > -{
> > - return 0;
> > -}
> > -
> > static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > struct ttm_mem_type_manager *man)
> > {
> > @@ -256,7 +251,6 @@ static void qxl_bo_move_notify(struct ttm_buffer_object
> > *bo,
> >
> > static struct ttm_bo_driver qxl_bo_driver = {
> > .ttm_tt_create = &qxl_ttm_tt_create,
> > - .invalidate_caches = &qxl_invalidate_caches,
> > .init_mem_type = &qxl_init_mem_type,
> > .eviction_valuable = ttm_bo_eviction_valuable,
> > .evict_flags = &qxl_evict_flags,
> > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
> > b/drivers/gpu/drm/radeon/radeon_ttm.c
> > index f4af67035673..40282bf0adbe 100644
> > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > @@ -66,11 +66,6 @@ static struct radeon_device *radeon_get_rdev(struct
> > ttm_bo_device *bdev)
> > return rdev;
> > }
> >
> > -static int radeon_invalidate_caches(struct ttm_bo_device *bdev, uint32_t
> > flags)
> > -{
> > - return 0;
> > -}
> > -
> > static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
> > struct ttm_mem_type_manager *man)
> > {
> > @@ -774,7 +769,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
> > .ttm_tt_create = &radeon_ttm_tt_create,
> > .ttm_tt_populate = &radeon_ttm_tt_populate,
> > .ttm_tt_unpopulate = &radeon_ttm_tt_unpopulate,
> > -
[PATCH RFC 3/3] drm/ttm: support memcg for ttm_tt
Charge TTM allocated system memory to memory cgroup which will
limit the memory usage of a group of processes.
The memory is always charged to the control group of task which
create this buffer object and when it's created. For example,
when a buffer is created by process A and exported to process B,
then process B populate this buffer, the memory is still charged
to process A's memcg; if a buffer is created by process A when in
memcg B, then A is moved to memcg C and populate this buffer, it
will charge memcg B.
Signed-off-by: Qiang Yu
---
drivers/gpu/drm/ttm/ttm_bo.c | 10 ++
drivers/gpu/drm/ttm/ttm_page_alloc.c | 18 +-
drivers/gpu/drm/ttm/ttm_tt.c | 3 +++
include/drm/ttm/ttm_bo_api.h | 5 +
include/drm/ttm/ttm_tt.h | 4
5 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 8d91b0428af1..4e64846ee523 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
static void ttm_bo_global_kobj_release(struct kobject *kobj);
@@ -162,6 +163,10 @@ static void ttm_bo_release_list(struct kref *list_kref)
if (!ttm_bo_uses_embedded_gem_object(bo))
dma_resv_fini(&bo->base._resv);
mutex_destroy(&bo->wu_mutex);
+#ifdef CONFIG_MEMCG
+ if (bo->memcg)
+ css_put(&bo->memcg->css);
+#endif
bo->destroy(bo);
ttm_mem_global_free(&ttm_mem_glob, acc_size);
}
@@ -1330,6 +1335,11 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
}
atomic_inc(&ttm_bo_glob.bo_count);
+#ifdef CONFIG_MEMCG
+ if (bo->type == ttm_bo_type_device)
+ bo->memcg = mem_cgroup_driver_get_from_current();
+#endif
+
/*
* For ttm_bo_type_device buffers, allocate
* address space from the device.
diff --git a/drivers/gpu/drm/ttm/ttm_page_alloc.c
b/drivers/gpu/drm/ttm/ttm_page_alloc.c
index b40a4678c296..ecd1831a1d38 100644
--- a/drivers/gpu/drm/ttm/ttm_page_alloc.c
+++ b/drivers/gpu/drm/ttm/ttm_page_alloc.c
@@ -42,7 +42,7 @@
#include /* for seq_printf */
#include
#include
-
+#include
#include
#include
@@ -1045,6 +1045,11 @@ ttm_pool_unpopulate_helper(struct ttm_tt *ttm, unsigned
mem_count_update)
ttm_put_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
ttm->caching_state);
ttm->state = tt_unpopulated;
+
+#ifdef CONFIG_MEMCG
+ if (ttm->memcg)
+ mem_cgroup_uncharge_drvmem(ttm->memcg, ttm->num_pages);
+#endif
}
int ttm_pool_populate(struct ttm_tt *ttm, struct ttm_operation_ctx *ctx)
@@ -1059,6 +1064,17 @@ int ttm_pool_populate(struct ttm_tt *ttm, struct
ttm_operation_ctx *ctx)
if (ttm_check_under_lowerlimit(mem_glob, ttm->num_pages, ctx))
return -ENOMEM;
+#ifdef CONFIG_MEMCG
+ if (ttm->memcg) {
+ gfp_t gfp_flags = GFP_USER;
+ if (ttm->page_flags & TTM_PAGE_FLAG_NO_RETRY)
+ gfp_flags |= __GFP_RETRY_MAYFAIL;
+ ret = mem_cgroup_charge_drvmem(ttm->memcg, gfp_flags,
ttm->num_pages);
+ if (ret)
+ return ret;
+ }
+#endif
+
ret = ttm_get_pages(ttm->pages, ttm->num_pages, ttm->page_flags,
ttm->caching_state);
if (unlikely(ret != 0)) {
diff --git a/drivers/gpu/drm/ttm/ttm_tt.c b/drivers/gpu/drm/ttm/ttm_tt.c
index e0e9b4f69db6..1acb153084e1 100644
--- a/drivers/gpu/drm/ttm/ttm_tt.c
+++ b/drivers/gpu/drm/ttm/ttm_tt.c
@@ -233,6 +233,9 @@ void ttm_tt_init_fields(struct ttm_tt *ttm, struct
ttm_buffer_object *bo,
ttm->state = tt_unpopulated;
ttm->swap_storage = NULL;
ttm->sg = bo->sg;
+#ifdef CONFIG_MEMCG
+ ttm->memcg = bo->memcg;
+#endif
}
int ttm_tt_init(struct ttm_tt *ttm, struct ttm_buffer_object *bo,
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 65e399d280f7..95a08e81a73e 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -54,6 +54,8 @@ struct ttm_place;
struct ttm_lru_bulk_move;
+struct mem_cgroup;
+
/**
* struct ttm_bus_placement
*
@@ -180,6 +182,9 @@ struct ttm_buffer_object {
void (*destroy) (struct ttm_buffer_object *);
unsigned long num_pages;
size_t acc_size;
+#ifdef CONFIG_MEMCG
+ struct mem_cgroup *memcg;
+#endif
/**
* Members not needing protection.
diff --git a/include/drm/ttm/ttm_tt.h b/include/drm/ttm/ttm_tt.h
index c0e928abf592..10fb5a557b95 100644
--- a/include/drm/ttm/ttm_tt.h
+++ b/include/drm/ttm/ttm_tt.h
@@ -33,6 +33,7 @@ struct ttm_tt;
struct ttm_mem_reg;
struct ttm_buffer_object;
struct ttm_operation_ctx;
+struct mem_cgroup;
#define TTM_PAGE_FLAG_WRITE (1 << 3)
#define TTM_PAGE_FLAG_SWAPPED (1 << 4)
@@ -116,6 +117,9 @@ struct ttm_tt {
tt_un
[PATCH RFC 2/3] mm: memcontrol: record driver memory statistics
Signed-off-by: Qiang Yu
---
include/linux/memcontrol.h | 1 +
mm/memcontrol.c| 9 +
2 files changed, 10 insertions(+)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index d76977943265..6518b4b5ee07 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -36,6 +36,7 @@ enum memcg_stat_item {
MEMCG_SOCK,
/* XXX: why are these zone and not node counters? */
MEMCG_KERNEL_STACK_KB,
+ MEMCG_DRV,
MEMCG_NR_STAT,
};
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 28595c276e6b..cdd3f3401598 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1413,6 +1413,9 @@ static char *memory_stat_format(struct mem_cgroup *memcg)
seq_buf_printf(&s, "sock %llu\n",
(u64)memcg_page_state(memcg, MEMCG_SOCK) *
PAGE_SIZE);
+ seq_buf_printf(&s, "driver %llu\n",
+ (u64)memcg_page_state(memcg, MEMCG_DRV) *
+ PAGE_SIZE);
seq_buf_printf(&s, "shmem %llu\n",
(u64)memcg_page_state(memcg, NR_SHMEM) *
@@ -6947,6 +6950,9 @@ EXPORT_SYMBOL(mem_cgroup_driver_get_from_current);
int mem_cgroup_charge_drvmem(struct mem_cgroup *memcg, gfp_t gfp,
unsigned long nr_pages)
{
+ if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
+ mod_memcg_state(memcg, MEMCG_DRV, nr_pages);
+
return try_charge(memcg, gfp, nr_pages);
}
EXPORT_SYMBOL(mem_cgroup_charge_drvmem);
@@ -6958,6 +6964,9 @@ EXPORT_SYMBOL(mem_cgroup_charge_drvmem);
*/
void mem_cgroup_uncharge_drvmem(struct mem_cgroup *memcg, unsigned long
nr_pages)
{
+ if (cgroup_subsys_on_dfl(memory_cgrp_subsys))
+ mod_memcg_state(memcg, MEMCG_DRV, -nr_pages);
+
refill_stock(memcg, nr_pages);
}
EXPORT_SYMBOL(mem_cgroup_uncharge_drvmem);
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC 1/3] mm: memcontrol: add mem_cgroup_(un)charge_drvmem
This is for driver which will allocate memory for both user application
and kernel device driver usage. For example, GPU driver will allocate
some GFP_USER pages and mapped to user to fill commands and data like
texture and vertex, then let GPU command processor "eat" these memory.
These buffers can be huge (offen several MB and may get to hundred
or even thousand MB).
Signed-off-by: Qiang Yu
---
include/linux/memcontrol.h | 21
mm/memcontrol.c| 49 ++
2 files changed, 70 insertions(+)
diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h
index ae703ea3ef48..d76977943265 100644
--- a/include/linux/memcontrol.h
+++ b/include/linux/memcontrol.h
@@ -1363,6 +1363,27 @@ static inline void memcg_set_shrinker_bit(struct
mem_cgroup *memcg,
}
#endif
+#ifdef CONFIG_MEMCG
+struct mem_cgroup *mem_cgroup_driver_get_from_current(void);
+int mem_cgroup_charge_drvmem(struct mem_cgroup *memcg, gfp_t gfp,
+unsigned long nr_pages);
+void mem_cgroup_uncharge_drvmem(struct mem_cgroup *memcg, unsigned long
nr_pages);
+#else
+static inline struct mem_cgroup *mem_cgroup_get_from_current(void)
+{
+ return NULL;
+}
+
+static inline int mem_cgroup_charge_drvmem(struct mem_cgroup *memcg, gfp_t gfp,
+ unsigned long nr_pages)
+{
+ return 0;
+}
+
+static inline void mem_cgroup_uncharge_drvmem(struct mem_cgroup *memcg,
+ unsigned long nr_pages) { }
+#endif
+
struct kmem_cache *memcg_kmem_get_cache(struct kmem_cache *cachep);
void memcg_kmem_put_cache(struct kmem_cache *cachep);
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 37592dd7ae32..28595c276e6b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -6913,6 +6913,55 @@ void mem_cgroup_uncharge_skmem(struct mem_cgroup *memcg,
unsigned int nr_pages)
refill_stock(memcg, nr_pages);
}
+/**
+ * mem_cgroup_driver_get_from_current - get memcg from current task for driver
+ *
+ * Return memcg from current task, NULL otherwise.
+ */
+struct mem_cgroup *mem_cgroup_driver_get_from_current(void)
+{
+ struct mem_cgroup *memcg, *ret = NULL;
+
+ if (mem_cgroup_disabled())
+ return NULL;
+
+ rcu_read_lock();
+ memcg = mem_cgroup_from_task(current);
+ if (memcg && memcg != root_mem_cgroup &&
+ css_tryget_online(&memcg->css))
+ ret = memcg;
+ rcu_read_unlock();
+
+ return ret;
+}
+EXPORT_SYMBOL(mem_cgroup_driver_get_from_current);
+
+/**
+ * mem_cgroup_charge_drvmem - charge a batch of pages for driver
+ * @memcg: memcg to charge
+ * @gfp: gfp flags for charge
+ * @nr_pages: number of pages to charge
+ *
+ * Return %true if success, %false otherwise.
+ */
+int mem_cgroup_charge_drvmem(struct mem_cgroup *memcg, gfp_t gfp,
+unsigned long nr_pages)
+{
+ return try_charge(memcg, gfp, nr_pages);
+}
+EXPORT_SYMBOL(mem_cgroup_charge_drvmem);
+
+/**
+ * mem_cgroup_uncharge_drvmem - uncharge a batch of pages for driver
+ * @memcg: memcg to uncharge
+ * @nr_pages: number of pages to uncharge
+ */
+void mem_cgroup_uncharge_drvmem(struct mem_cgroup *memcg, unsigned long
nr_pages)
+{
+ refill_stock(memcg, nr_pages);
+}
+EXPORT_SYMBOL(mem_cgroup_uncharge_drvmem);
+
static int __init cgroup_memory(char *s)
{
char *token;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH RFC 0/3] mm/memcontrol drm/ttm: charge ttm buffer backed by system memory
Buffers created by GPU driver could be huge (often several MB and even hundred or thousand MB). Some GPU driver call drm_gem_get_pages() which uses shmem to allocate these buffers which will charge memcg already, while some GPU driver like amdgpu use TTM which just allocate these system memory backed buffers with alloc_pages() so won't charge memcg currently. Not like pure kernel memory, GPU buffer need to be mapped to user space for user filling data and command then let GPU hardware consume these buffers. So it is not proper to use memcg kmem by adding __GFP_ACCOUNT to alloc_pages gfp flags. Another reason is back memory of GPU buffer may be allocated latter after the buffer object is created, and even in other processes. So we need to record the memcg when buffer object creation, then charge it latter when needed. TTM will use a page pool acting as a cache for write-combine/no-cache pages. So adding new GFP flags for alloc_pages also does not work. Qiang Yu (3): mm: memcontrol: add mem_cgroup_(un)charge_drvmem mm: memcontrol: record driver memory statistics drm/ttm: support memcg for ttm_tt drivers/gpu/drm/ttm/ttm_bo.c | 10 + drivers/gpu/drm/ttm/ttm_page_alloc.c | 18 - drivers/gpu/drm/ttm/ttm_tt.c | 3 ++ include/drm/ttm/ttm_bo_api.h | 5 +++ include/drm/ttm/ttm_tt.h | 4 ++ include/linux/memcontrol.h | 22 +++ mm/memcontrol.c | 58 7 files changed, 119 insertions(+), 1 deletion(-) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] pci: prevent putting nvidia GPUs into lower device states on certain intel bridges
okay.. so checking whatever is the difference with _REV being 5 (meaning the firmware uses the legacy paths) doesn't help in any way. It's using a different method to turn the link of and the other ACPI variables touched either point to undocumented registers on the PCI bridge or internal ACPI memory... so, anybody with any other ideas? I really wished the nvidia driver would enable runpm on pre turing GPUs, but that's sadly not the case and on Turing things seem to be totally different, so it wouldn't help to check there as well... *sigh* On Tue, Dec 10, 2019 at 9:49 PM Karol Herbst wrote: > > On Tue, Dec 10, 2019 at 8:58 PM Dave Airlie wrote: > > > > On Mon, 9 Dec 2019 at 21:39, Rafael J. Wysocki wrote: > > > > > > On Mon, Dec 9, 2019 at 12:17 PM Karol Herbst wrote: > > > > > > > > anybody any other ideas? > > > > > > Not yet, but I'm trying to collect some more information. > > > > > > > It seems that both patches don't really fix > > > > the issue and I have no idea left on my side to try out. The only > > > > thing left I could do to further investigate would be to reverse > > > > engineer the Nvidia driver as they support runpm on Turing+ GPUs now, > > > > but I've heard users having similar issues to the one Lyude told us > > > > about... and I couldn't verify that the patches help there either in a > > > > reliable way. > > > > > > It looks like the newer (8+) versions of Windows expect the GPU driver > > > to prepare the GPU for power removal in some specific way and the > > > latter fails if the GPU has not been prepared as expected. > > > > > > Because testing indicates that the Windows 7 path in the platform > > > firmware works, it may be worth trying to do what it does to the PCIe > > > link before invoking the _OFF method for the power resource > > > controlling the GPU power. > > > > > > > Remember the pre Win8 path required calling a DSM method to actually > > power the card down, I think by the time we reach these methods in > > those cases the card is already gone. > > > > Dave. > > > > The point was that the firmware seems to do more in the legacy paths > and maybe we just have to do those things inside the driver instead > when using the new method. Also the _DSM call just wraps around the > interfaces on newer firmware anyway. The OS check is usually what > makes the difference. I might be wrong about the _DSM call just > wrapping though, but I think I saw it at least in some firmware at > some point. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] video: fbdev: nvidia: clean up indentation issues and comment block
From: Colin Ian King
There is a hunk of code that is incorrectly indented, clean up the
indentation and a comment block. Also remove block braces around a
one line statement on an if condition and add missing spaces after
if keywords.
Signed-off-by: Colin Ian King
---
drivers/video/fbdev/nvidia/nvidia.c | 41 ++---
1 file changed, 20 insertions(+), 21 deletions(-)
diff --git a/drivers/video/fbdev/nvidia/nvidia.c
b/drivers/video/fbdev/nvidia/nvidia.c
index c583c018304d..c24de9107958 100644
--- a/drivers/video/fbdev/nvidia/nvidia.c
+++ b/drivers/video/fbdev/nvidia/nvidia.c
@@ -168,27 +168,26 @@ static int nvidia_panel_tweak(struct nvidia_par *par,
{
int tweak = 0;
- if (par->paneltweak) {
- tweak = par->paneltweak;
- } else {
- /* begin flat panel hacks */
- /* This is unfortunate, but some chips need this register
- tweaked or else you get artifacts where adjacent pixels are
- swapped. There are no hard rules for what to set here so all
- we can do is experiment and apply hacks. */
-
- if(((par->Chipset & 0x) == 0x0328) && (state->bpp == 32)) {
- /* At least one NV34 laptop needs this workaround. */
- tweak = -1;
- }
-
- if((par->Chipset & 0xfff0) == 0x0310) {
- tweak = 1;
- }
- /* end flat panel hacks */
- }
-
- return tweak;
+ if (par->paneltweak) {
+ tweak = par->paneltweak;
+ } else {
+ /* Begin flat panel hacks.
+* This is unfortunate, but some chips need this register
+* tweaked or else you get artifacts where adjacent pixels are
+* swapped. There are no hard rules for what to set here so all
+* we can do is experiment and apply hacks.
+*/
+ if (((par->Chipset & 0x) == 0x0328) && (state->bpp == 32)) {
+ /* At least one NV34 laptop needs this workaround. */
+ tweak = -1;
+ }
+
+ if ((par->Chipset & 0xfff0) == 0x0310)
+ tweak = 1;
+ /* end flat panel hacks */
+ }
+
+ return tweak;
}
static void nvidia_screen_off(struct nvidia_par *par, int on)
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: nuke invalidate_caches callback
Ping? Just a trivial cleanup.
Am 10.01.20 um 16:09 schrieb Christian König:
Another completely unused feature.
Signed-off-by: Christian König
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 6 --
drivers/gpu/drm/nouveau/nouveau_bo.c | 8
drivers/gpu/drm/qxl/qxl_ttm.c | 6 --
drivers/gpu/drm/radeon/radeon_ttm.c| 6 --
drivers/gpu/drm/ttm/ttm_bo.c | 9 +
drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 6 --
include/drm/ttm/ttm_bo_driver.h| 15 ---
7 files changed, 1 insertion(+), 55 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 445de594c214..7c4b1cbd9a50 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -68,11 +68,6 @@ static int amdgpu_map_buffer(struct ttm_buffer_object *bo,
static int amdgpu_ttm_debugfs_init(struct amdgpu_device *adev);
static void amdgpu_ttm_debugfs_fini(struct amdgpu_device *adev);
-static int amdgpu_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
-{
- return 0;
-}
-
/**
* amdgpu_init_mem_type - Initialize a memory manager for a specific type of
* memory request.
@@ -1637,7 +1632,6 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
.ttm_tt_create = &amdgpu_ttm_tt_create,
.ttm_tt_populate = &amdgpu_ttm_tt_populate,
.ttm_tt_unpopulate = &amdgpu_ttm_tt_unpopulate,
- .invalidate_caches = &amdgpu_invalidate_caches,
.init_mem_type = &amdgpu_init_mem_type,
.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
.evict_flags = &amdgpu_evict_flags,
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index f8015e0318d7..81668104595f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -646,13 +646,6 @@ nouveau_ttm_tt_create(struct ttm_buffer_object *bo,
uint32_t page_flags)
return nouveau_sgdma_create_ttm(bo, page_flags);
}
-static int
-nouveau_bo_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
-{
- /* We'll do this from user space. */
- return 0;
-}
-
static int
nouveau_bo_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
struct ttm_mem_type_manager *man)
@@ -1696,7 +1689,6 @@ struct ttm_bo_driver nouveau_bo_driver = {
.ttm_tt_create = &nouveau_ttm_tt_create,
.ttm_tt_populate = &nouveau_ttm_tt_populate,
.ttm_tt_unpopulate = &nouveau_ttm_tt_unpopulate,
- .invalidate_caches = nouveau_bo_invalidate_caches,
.init_mem_type = nouveau_bo_init_mem_type,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = nouveau_bo_evict_flags,
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 16a5e903533d..62a5e424971b 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -48,11 +48,6 @@ static struct qxl_device *qxl_get_qdev(struct ttm_bo_device
*bdev)
return qdev;
}
-static int qxl_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
-{
- return 0;
-}
-
static int qxl_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
struct ttm_mem_type_manager *man)
{
@@ -256,7 +251,6 @@ static void qxl_bo_move_notify(struct ttm_buffer_object *bo,
static struct ttm_bo_driver qxl_bo_driver = {
.ttm_tt_create = &qxl_ttm_tt_create,
- .invalidate_caches = &qxl_invalidate_caches,
.init_mem_type = &qxl_init_mem_type,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &qxl_evict_flags,
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index f4af67035673..40282bf0adbe 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -66,11 +66,6 @@ static struct radeon_device *radeon_get_rdev(struct
ttm_bo_device *bdev)
return rdev;
}
-static int radeon_invalidate_caches(struct ttm_bo_device *bdev, uint32_t flags)
-{
- return 0;
-}
-
static int radeon_init_mem_type(struct ttm_bo_device *bdev, uint32_t type,
struct ttm_mem_type_manager *man)
{
@@ -774,7 +769,6 @@ static struct ttm_bo_driver radeon_bo_driver = {
.ttm_tt_create = &radeon_ttm_tt_create,
.ttm_tt_populate = &radeon_ttm_tt_populate,
.ttm_tt_unpopulate = &radeon_ttm_tt_unpopulate,
- .invalidate_caches = &radeon_invalidate_caches,
.init_mem_type = &radeon_init_mem_type,
.eviction_valuable = ttm_bo_eviction_valuable,
.evict_flags = &radeon_evict_flags,
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 5df596fb0280..06f6d650827f 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -372,14 +372,7 @@ static int ttm_bo_handle_move_mem(struc
[PATCH RESEND] drm/nouveau: Add HD-audio component notifier support
This patch adds the support for the notification of HD-audio hotplug
via the already existing drm_audio_component framework. This allows
us more reliable hotplug notification and ELD transfer without
accessing HD-audio bus; it's more efficient, and more importantly, it
works without waking up the runtime PM.
The implementation is rather simplistic: nouveau driver provides the
get_eld ops for HD-audio, and it notifies the audio hotplug via
pin_eld_notify callback upon each nv50_audio_enable() and _disable()
call. As the HD-audio pin assignment seems corresponding to the CRTC,
the crtc->index number is passed directly as the zero-based port
number.
The bind and unbind callbacks handle the device-link so that it
assures the PM call order.
Link: https://lore.kernel.org/r/20190722143815.7339-3-ti...@suse.de
Reviewed-by: Lyude Paul
Cc: Jaroslav Kysela
Signed-off-by: Takashi Iwai
---
This is a re-submission of the same patch in the last year, applied on
top of the current linux-next.
Without this audio component support, the HD-audio HDMI codec won't
recognize the display hotplug at all due to the recent runtime PM
change. So, please review and merge if it's OK. Thanks.
drivers/gpu/drm/nouveau/Kconfig | 1 +
drivers/gpu/drm/nouveau/dispnv50/disp.c | 111
drivers/gpu/drm/nouveau/nouveau_drv.h | 7 ++
3 files changed, 119 insertions(+)
diff --git a/drivers/gpu/drm/nouveau/Kconfig b/drivers/gpu/drm/nouveau/Kconfig
index 9c990266e876..7b5c4121374a 100644
--- a/drivers/gpu/drm/nouveau/Kconfig
+++ b/drivers/gpu/drm/nouveau/Kconfig
@@ -16,6 +16,7 @@ config DRM_NOUVEAU
select INPUT if ACPI && X86
select THERMAL if ACPI && X86
select ACPI_VIDEO if ACPI && X86
+ select SND_HDA_COMPONENT if SND_HDA_CORE
help
Choose this option for open-source NVIDIA support.
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index fd31bff0c920..368749a883a1 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -29,6 +29,7 @@
#include
#include
+#include
#include
#include
@@ -476,12 +477,113 @@ nv50_dac_create(struct drm_connector *connector, struct
dcb_output *dcbe)
return 0;
}
+/*
+ * audio component binding for ELD notification
+ */
+static void
+nv50_audio_component_eld_notify(struct drm_audio_component *acomp, int port)
+{
+ if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify)
+ acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
+port, -1);
+}
+
+static int
+nv50_audio_component_get_eld(struct device *kdev, int port, int pipe,
+bool *enabled, unsigned char *buf, int max_bytes)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(kdev);
+ struct nouveau_drm *drm = nouveau_drm(drm_dev);
+ struct drm_encoder *encoder;
+ struct nouveau_encoder *nv_encoder;
+ struct nouveau_connector *nv_connector;
+ struct nouveau_crtc *nv_crtc;
+ int ret = 0;
+
+ *enabled = false;
+ drm_for_each_encoder(encoder, drm->dev) {
+ nv_encoder = nouveau_encoder(encoder);
+ nv_connector = nouveau_encoder_connector_get(nv_encoder);
+ nv_crtc = nouveau_crtc(encoder->crtc);
+ if (!nv_connector || !nv_crtc || nv_crtc->index != port)
+ continue;
+ *enabled = drm_detect_monitor_audio(nv_connector->edid);
+ if (*enabled) {
+ ret = drm_eld_size(nv_connector->base.eld);
+ memcpy(buf, nv_connector->base.eld,
+ min(max_bytes, ret));
+ }
+ break;
+ }
+ return ret;
+}
+
+static const struct drm_audio_component_ops nv50_audio_component_ops = {
+ .get_eld = nv50_audio_component_get_eld,
+};
+
+static int
+nv50_audio_component_bind(struct device *kdev, struct device *hda_kdev,
+ void *data)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(kdev);
+ struct nouveau_drm *drm = nouveau_drm(drm_dev);
+ struct drm_audio_component *acomp = data;
+
+ if (WARN_ON(!device_link_add(hda_kdev, kdev, DL_FLAG_STATELESS)))
+ return -ENOMEM;
+
+ drm_modeset_lock_all(drm_dev);
+ acomp->ops = &nv50_audio_component_ops;
+ acomp->dev = kdev;
+ drm->audio.component = acomp;
+ drm_modeset_unlock_all(drm_dev);
+ return 0;
+}
+
+static void
+nv50_audio_component_unbind(struct device *kdev, struct device *hda_kdev,
+ void *data)
+{
+ struct drm_device *drm_dev = dev_get_drvdata(kdev);
+ struct nouveau_drm *drm = nouveau_drm(drm_dev);
+ struct drm_audio_component *acomp = data;
+
+ drm_modeset_lock_all(drm_dev);
+ drm->audio.component = NUL
[PATCH] drm/amdgpu/display: Use u64 divide macro for round up division
From: Mikita Lipski [why] Fix compilation warnings on i386 architecture: undefined reference to `__udivdi3' [how] Switch DIV_ROUND_UP to DIV64_U64_ROUND_UP Reported-by: Randy Dunlap Signed-off-by: Mikita Lipski --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c index 52fb207393ef..96b391e4b3e7 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c @@ -534,7 +534,7 @@ static int kbps_to_peak_pbn(int kbps) peak_kbps *= 1006; peak_kbps = div_u64(peak_kbps, 1000); - return (int) DIV_ROUND_UP(peak_kbps * 64, (54 * 8 * 1000)); + return (int) DIV64_U64_ROUND_UP(peak_kbps * 64, (54 * 8 * 1000)); } static void set_dsc_configs_from_fairness_vars(struct dsc_mst_fairness_params *params, -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH][next] drm/gma500: clean up two indentation issues
From: Colin Ian King
There are a couple of statements that are indented too deeply,
clean these up by removing the extraneous tabs. Also remove an
empty line.
Signed-off-by: Colin Ian King
---
drivers/gpu/drm/gma500/cdv_intel_dp.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/gma500/cdv_intel_dp.c
b/drivers/gpu/drm/gma500/cdv_intel_dp.c
index 5772b2dce0d6..b5360bd405c5 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_dp.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_dp.c
@@ -1529,8 +1529,7 @@ cdv_intel_dp_start_link_train(struct gma_encoder *encoder)
clock_recovery = false;
DRM_DEBUG_KMS("Start train\n");
- reg = DP | DP_LINK_TRAIN_PAT_1;
-
+ reg = DP | DP_LINK_TRAIN_PAT_1;
for (;;) {
/* Use intel_dp->train_set[0] to set the voltage and pre
emphasis values */
@@ -1603,7 +1602,7 @@ cdv_intel_dp_complete_link_train(struct gma_encoder
*encoder)
cr_tries = 0;
DRM_DEBUG_KMS("\n");
- reg = DP | DP_LINK_TRAIN_PAT_2;
+ reg = DP | DP_LINK_TRAIN_PAT_2;
for (;;) {
--
2.24.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: Add missing vmalloc header
Am Dienstag, 31. Dezember 2019, 09:12:36 CET schrieb Krzysztof Kozlowski: > The Rockship DRM GEM code uses vmap()/vunmap() so vmalloc header must be > included to avoid warnings like (on IA64, compile tested): > > drivers/gpu/drm/rockchip/rockchip_drm_gem.c: In function > ‘rockchip_gem_alloc_iommu’: > drivers/gpu/drm/rockchip/rockchip_drm_gem.c:134:20: error: > implicit declaration of function ‘vmap’ > [-Werror=implicit-function-declaration] > > Reported-by: kbuild test robot > Signed-off-by: Krzysztof Kozlowski applied to drm-misc-next Thanks Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH next] drm/i915: fix build error without ACPI
On Mon, 13 Jan 2020, Chen Zhou wrote: > If CONFIG_ACPI=n and CONFIG_BACKLIGHT_CLASS_DEVICE=m, compilation complains > with undefined references: > > drivers/gpu/drm/i915/display/intel_panel.o: In function > `intel_backlight_device_register': > intel_panel.c:(.text+0x4dd9): undefined reference to > `backlight_device_register' > drivers/gpu/drm/i915/display/intel_panel.o: In function > `intel_backlight_device_unregister': > intel_panel.c:(.text+0x4e96): undefined reference to > `backlight_device_unregister' > > This patch select BACKLIGHT_CLASS_DEVICE directly. i915 does not unconditionally require backlight. See e.g. [1] for the details. BR, Jani. [1] http://lore.kernel.org/r/87o8veotf9@intel.com > > Reported-by: Hulk Robot > Signed-off-by: Chen Zhou > --- > drivers/gpu/drm/i915/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index ba95959..6b69dab 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -16,7 +16,7 @@ config DRM_I915 > select IRQ_WORK > # i915 depends on ACPI_VIDEO when ACPI is enabled > # but for select to work, need to select ACPI_VIDEO's dependencies, ick > - select BACKLIGHT_CLASS_DEVICE if ACPI > + select BACKLIGHT_CLASS_DEVICE > select INPUT if ACPI > select ACPI_VIDEO if ACPI > select ACPI_BUTTON if ACPI -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v24 1/2] Documentation: bridge: Add documentation for ps8640 DT properties
On Mon, Dec 30, 2019 at 3:04 AM Enric Balletbo i Serra wrote: > > From: Jitao Shi > > Add documentation for DT properties supported by > ps8640 DSI-eDP converter. > > Signed-off-by: Jitao Shi > Acked-by: Rob Herring > Reviewed-by: Philipp Zabel > Signed-off-by: Ulrich Hecht > Signed-off-by: Enric Balletbo i Serra > --- > I maintained the ack from Rob Herring and the review from Philipp > because in essence the only thing I did is migrate to YAML format and > check that no errors are reported via dtbs_check. Just let me know if > you're not agree. Reviewed-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/bridge: tfp410: add pclk limits
Add pixel clock limits to the driver as per TFP410 datasheet: min 25MHz,
max 165MHz.
Signed-off-by: Tomi Valkeinen
---
drivers/gpu/drm/bridge/ti-tfp410.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c
b/drivers/gpu/drm/bridge/ti-tfp410.c
index 6f6d6d1e60ae..f378a8f79bcb 100644
--- a/drivers/gpu/drm/bridge/ti-tfp410.c
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -167,10 +167,20 @@ static void tfp410_disable(struct drm_bridge *bridge)
gpiod_set_value_cansleep(dvi->powerdown, 1);
}
+static enum drm_mode_status tfp410_mode_valid(struct drm_bridge *bridge,
+ const struct drm_display_mode
*mode)
+{
+ if (mode->clock < 25000 || mode->clock > 165000)
+ return MODE_BAD;
+
+ return MODE_OK;
+}
+
static const struct drm_bridge_funcs tfp410_bridge_funcs = {
.attach = tfp410_attach,
.enable = tfp410_enable,
.disable= tfp410_disable,
+ .mode_valid = tfp410_mode_valid,
};
static void tfp410_hpd_work_func(struct work_struct *work)
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/rockchip: use DIV_ROUND_UP macro for calculations.
Am Donnerstag, 9. Januar 2020, 15:20:57 CET schrieb Wambui Karuga: > Replace the open coded calculation with the more concise and readable > DIV_ROUND_UP macro. > > Signed-off-by: Wambui Karuga applied to drm-misc-next Thanks Heiko ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/1] drm/rockchip: fix integer type used for storing dp data rate
Am Donnerstag, 9. Januar 2020, 08:31:29 CET schrieb Tobias Schramm:
> commmit 2589c4025f13 ("drm/rockchip: Avoid drm_dp_link helpers") changes
> the type of variables used to store the display port data rate and
> number of lanes to u8. However u8 is not sufficient to store the link
> data rate of the display port.
> This commit reverts the type of data rate to unsigned int.
>
> Signed-off-by: Tobias Schramm
applied to drm-misc-fixes
Thanks
Heiko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/bridge: tc358767: fix poll timeouts
Hi Andrzej,
On 09/12/2019 16:45, Andrey Smirnov wrote:
+ Chris Healy
On Mon, Dec 9, 2019 at 12:27 AM Tomi Valkeinen wrote:
Link training fails with:
Link training timeout waiting for LT_LOOPDONE!
main link enable error: -110
This is caused by too tight timeouts, which were changed recently in
aa92213f388b ("drm/bridge: tc358767: Simplify polling in tc_link_training()").
With a quick glance, the commit does not change the timeouts. However,
the method of delaying/sleeping is different, and as the timeout in the
previous implementation was not explicit, the new version in practice
has much tighter timeout.
The same change was made to other parts in the driver, but the link
training timeout is the only one I have seen causing issues.
Nevertheless, 1 us sleep is not very sane, and the timeouts look pretty
tight, so lets fix all the timeouts.
One exception was the aux busy poll, where the poll sleep was much
longer than necessary (or optimal).
I measured the times on my setup, and now the sleep times are set to
such values that they result in multiple loops, but not too many (say,
5-10 loops). The timeouts were all increased to 100ms, which should be
more than enough for all of these, but in case of bad errors, shouldn't
stop the driver as multi-second timeouts could do.
Signed-off-by: Tomi Valkeinen
Fixes: aa92213f388b ("drm/bridge: tc358767: Simplify polling in
tc_link_training()")
Tested on RDU2 with TC358767/eDP panel, doesn't seem to break anything
there, so:
Tested-by: Andrey Smirnov
Andrzej, can you pick this up for -fixes?
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206141] VCE UVD ring test failed -110
https://bugzilla.kernel.org/show_bug.cgi?id=206141 --- Comment #2 from Janpieter Sollie (janpieter.sol...@dommel.be) --- Tried to make ALL functions where ETIMEDOUT is specified (in drivers/gpu/amd/amdgpu/) with timeout << 2, but nothing. Am I looking at the wrong function here? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: potential information leak in do_fb_ioctl()
On Fri, Jan 3, 2020 at 2:09 PM Bartlomiej Zolnierkiewicz wrote: > On 10/29/19 8:02 PM, Eric W. Biederman wrote: > > > > The goal is to avoid memory that has values of the previous users of > > that memory region from leaking to userspace. Which depending on who > > the previous user of that memory region is could tell userspace > > information about what the kernel is doing that it should not be allowed > > to find out. > > > > I tried to trace through where "info" and thus presumably "info->fix" is > > coming from and only made it as far as register_framebuffer. Given > > "info" (and thus "info->fix") comes from framebuffer_alloc() (which is > called by fbdev device drivers prior to registering "info" with > register_framebuffer()). framebuffer_alloc() does kzalloc() on "info". > > Therefore shouldn't memcpy() (as suggested by Jeo Perches) be enough? Is it guaranteed that all drivers call framebuffer_alloc() rather than open-coding it somewhere? Here is a list of all files that call register_framebuffer() without first calling framebuffer_alloc: $ git grep -wl register_framebuffer | xargs grep -L framebuffer_alloc Documentation/fb/framebuffer.rst drivers/media/pci/ivtv/ivtvfb.c drivers/media/platform/vivid/vivid-osd.c drivers/video/fbdev/68328fb.c drivers/video/fbdev/acornfb.c drivers/video/fbdev/amba-clcd.c drivers/video/fbdev/atafb.c drivers/video/fbdev/au1100fb.c drivers/video/fbdev/controlfb.c drivers/video/fbdev/core/fbmem.c drivers/video/fbdev/cyber2000fb.c drivers/video/fbdev/fsl-diu-fb.c drivers/video/fbdev/g364fb.c drivers/video/fbdev/goldfishfb.c drivers/video/fbdev/hpfb.c drivers/video/fbdev/macfb.c drivers/video/fbdev/matrox/matroxfb_base.c drivers/video/fbdev/matrox/matroxfb_crtc2.c drivers/video/fbdev/maxinefb.c drivers/video/fbdev/ocfb.c drivers/video/fbdev/pxafb.c drivers/video/fbdev/sa1100fb.c drivers/video/fbdev/stifb.c drivers/video/fbdev/valkyriefb.c drivers/video/fbdev/vermilion/vermilion.c drivers/video/fbdev/vt8500lcdfb.c drivers/video/fbdev/wm8505fb.c drivers/video/fbdev/xilinxfb.c It's possible (even likely, the ones I looked at are fine) that they all correctly zero out the fb_info structure first, but it seems hard to guarantee, so Eric's suggestion would possibly still be the safer choice. Arnd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 0/6]: drm: Introduce struct drm_device based WARN* and use them in i915
On Mon, 13 Jan 2020, Pankaj Bharadiya wrote: > Device specific dev_WARN and dev_WARN_ONCE macros available in kernel > include device information in the backtrace, so we know what device > the warnings originate from. > > Similar to this, add new struct drm_device based drm_WARN* macros. These > macros include device information in the backtrace, so we know > what device the warnings originate from. Knowing the device specific > information in the backtrace would be helpful in development all > around. > > This patch series aims to convert calls of WARN(), WARN_ON(), > WARN_ONCE() and WARN_ON_ONCE() in i916 driver to use the drm > device-specific variants automatically wherever struct device pointer > is available. > > To do this, this patch series - > - introduces new struct drm_device based WARN* macros > - automatically converts WARN* with device specific dev_WARN* > variants using coccinelle semantic patch scripts. > > The goal is to convert all the calls of WARN* with drm_WARN* in i915, > but there are still cases where device pointer is not readily > available in some functions (or I missed them somehow) using WARN* > hence some manual churning is needed. Handle such remaining cases > separately later. > > Changes since RFC at [1] > - Introduce drm_WARN* macros and use them as suggested by Sam and Jani > - Get rid of extra local variables > > [1] https://patchwork.freedesktop.org/series/71668/ > > Pankaj Bharadiya (6): > drm/print: introduce new struct drm_device based WARN* macros > drm/i915: add helper functions to get device ptr > drm/i915: Make WARN* drm specific where drm_device ptr available > drm/i915: Make WARN* drm specific where drm_priv ptr is available > drm/i915: Make WARN* drm specific where dev_priv can be extracted. > drm/i915: Make WARN* drm specific for various cases. I think the last four patches actually making the conversions are too big, and need to be split out to smaller pieces. These are bound to conflict in annoying ways. The absolute minimum I think it splitting by directory, and in particular the changes to gvt needs to be handled by gvt maintainers. BR, Jani. > > drivers/gpu/drm/i915/display/icl_dsi.c| 14 +- > drivers/gpu/drm/i915/display/intel_atomic.c | 6 +- > drivers/gpu/drm/i915/display/intel_audio.c| 19 +- > drivers/gpu/drm/i915/display/intel_bios.c | 10 +- > drivers/gpu/drm/i915/display/intel_bw.c | 3 +- > drivers/gpu/drm/i915/display/intel_cdclk.c| 81 +++--- > drivers/gpu/drm/i915/display/intel_color.c| 3 +- > .../gpu/drm/i915/display/intel_combo_phy.c| 2 +- > .../gpu/drm/i915/display/intel_connector.c| 3 +- > drivers/gpu/drm/i915/display/intel_crt.c | 10 +- > drivers/gpu/drm/i915/display/intel_ddi.c | 100 > drivers/gpu/drm/i915/display/intel_display.c | 239 ++ > .../drm/i915/display/intel_display_power.c| 170 +++-- > .../drm/i915/display/intel_display_types.h| 14 + > drivers/gpu/drm/i915/display/intel_dp.c | 121 + > drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 +- > drivers/gpu/drm/i915/display/intel_dpio_phy.c | 3 +- > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 39 +-- > drivers/gpu/drm/i915/display/intel_dsb.c | 6 +- > .../i915/display/intel_dsi_dcs_backlight.c| 2 +- > drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 5 +- > drivers/gpu/drm/i915/display/intel_fbc.c | 23 +- > drivers/gpu/drm/i915/display/intel_fbdev.c| 13 +- > drivers/gpu/drm/i915/display/intel_gmbus.c| 3 +- > drivers/gpu/drm/i915/display/intel_hdcp.c | 21 +- > drivers/gpu/drm/i915/display/intel_hdmi.c | 71 +++--- > drivers/gpu/drm/i915/display/intel_hotplug.c | 7 +- > .../gpu/drm/i915/display/intel_lpe_audio.c| 2 +- > drivers/gpu/drm/i915/display/intel_lvds.c | 7 +- > drivers/gpu/drm/i915/display/intel_opregion.c | 7 +- > drivers/gpu/drm/i915/display/intel_overlay.c | 18 +- > drivers/gpu/drm/i915/display/intel_panel.c| 21 +- > drivers/gpu/drm/i915/display/intel_pipe_crc.c | 7 +- > drivers/gpu/drm/i915/display/intel_psr.c | 32 ++- > drivers/gpu/drm/i915/display/intel_sdvo.c | 17 +- > drivers/gpu/drm/i915/display/intel_sprite.c | 5 +- > drivers/gpu/drm/i915/display/intel_tc.c | 17 +- > drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +- > drivers/gpu/drm/i915/display/vlv_dsi.c| 2 +- > .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 4 +- > drivers/gpu/drm/i915/gem/i915_gem_pm.c| 4 +- > drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 3 +- > drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 13 +- > drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 15 +- > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 10 +- > drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 +- > drivers/gpu/drm/i915/gt/intel_gt_irq.c| 15 +- > drivers/gpu/drm/i915/gt/intel_gt_pm_irq.c | 2 +- >
Re: [Intel-gfx] [PATCH 1/6] drm/print: introduce new struct drm_device based WARN* macros
On Mon, 13 Jan 2020, Pankaj Bharadiya
wrote:
> Add new struct drm_device based WARN* macros. These are modeled after
> the core kernel device based WARN* macros. These would be preferred
> over the regular WARN* macros, where possible.
>
> These macros include device information in the backtrace, so we know
> what device the warnings originate from.
>
> Knowing the device specific information in the backtrace would be
> helpful in development all around.
>
> Signed-off-by: Pankaj Bharadiya
> ---
> include/drm/drm_print.h | 30 ++
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 8f99d389792d..61a7224e697d 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -553,4 +553,34 @@ void __drm_err(const char *format, ...);
> #define DRM_DEBUG_PRIME_RATELIMITED(fmt, ...)
> \
> DRM_DEV_DEBUG_PRIME_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
>
> +/*
> + * struct drm_device based WARNs
> + *
> + *
> + * drm_WARN*() acts like WARN*(), but with the key difference of
> + * using device specific information so that we know from which device
> + * warning is originating from.
> + *
> + * Prefer drm_device based dev_WARN* over regular WARN*
drm_WARN, not dev_WARN.
I don't really appreciated changing case mid-symbol, but that matches
dev_WARN. :/
Other than that,
Acked-by: Jani Nikula
> + */
> +
> +/* Helper for struct drm_device based WARNs */
> +#define drm_WARN(drm, condition, format, arg...) \
> + WARN(condition, "%s %s: " format, \
> + dev_driver_string((drm)->dev), \
> + dev_name((drm)->dev), ## arg)
> +
> +#define drm_WARN_ONCE(drm, condition, format, arg...)
> \
> + WARN_ONCE(condition, "%s %s: " format, \
> + dev_driver_string((drm)->dev), \
> + dev_name((drm)->dev), ## arg)
> +
> +#define drm_WARN_ON(drm, x) \
> + drm_WARN((drm), (x), "%s", \
> + "drm_WARN_ON(" __stringify(x) ")")
> +
> +#define drm_WARN_ON_ONCE(drm, x) \
> + drm_WARN_ONCE((drm), (x), "%s", \
> + "drm_WARN_ON_ONCE(" __stringify(x) ")")
> +
> #endif /* DRM_PRINT_H_ */
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 2/6] drm/i915: add helper functions to get device ptr
On Mon, 13 Jan 2020, Pankaj Bharadiya
wrote:
> We will need struct drm_device pointer to pass it to drm_WARN* calls.
>
> Add helper functions to exract drm_device pointer from various structs.
Please don't do this.
We use the helpers we currently have when they involve something more
complex than dereferences, such as container_of() or having to use
dev_get_drvdata() or pci_get_drvdata().
I really don't want people to use e.g.
i915_to_dev(i915)
over
&i915->drm
BR,
Jani.
>
> Signed-off-by: Pankaj Bharadiya
> ---
> drivers/gpu/drm/i915/display/intel_display_types.h | 14 ++
> drivers/gpu/drm/i915/gvt/gvt.h | 5 +
> drivers/gpu/drm/i915/i915_drv.h| 11 +++
> 3 files changed, 30 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 7964b3dc0046..765878718fe9 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1521,6 +1521,20 @@ dp_to_i915(struct intel_dp *intel_dp)
> return to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> }
>
> +static inline struct drm_device *enc_to_dev(const struct intel_encoder
> *encoder)
> +{
> + return encoder->base.dev;
> +}
> +
> +static inline struct drm_device *
> +crtc_state_to_dev(const struct intel_crtc_state *state)
> +{
> + struct intel_crtc *crtc = to_intel_crtc(state->uapi.crtc);
> + struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> +
> + return i915_to_dev(i915);
> +}
> +
> static inline struct intel_digital_port *
> hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
> {
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index b47c6acaf9c0..d257399b075a 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -348,6 +348,11 @@ static inline struct intel_gvt *to_gvt(struct
> drm_i915_private *i915)
> return i915->gvt;
> }
>
> +static inline struct drm_device *vgpu_to_dev(const struct intel_vgpu *vgpu)
> +{
> + return i915_to_dev(vgpu->gvt->dev_priv);
> +}
> +
> enum {
> INTEL_GVT_REQUEST_EMULATE_VBLANK = 0,
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e7be4c3e43c6..bdc89d021ff8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1325,6 +1325,17 @@ static inline struct drm_i915_private
> *pdev_to_i915(struct pci_dev *pdev)
> return pci_get_drvdata(pdev);
> }
>
> +static inline struct drm_device *i915_to_dev(struct drm_i915_private *i915)
> +{
> + return &i915->drm;
> +}
> +
> +static inline struct drm_device *
> +perf_stream_to_dev(const struct i915_perf_stream *stream)
> +{
> + return i915_to_dev(stream->perf->i915);
> +}
> +
> /* Simple iterator over all initialised engines */
> #define for_each_engine(engine__, dev_priv__, id__) \
> for ((id__) = 0; \
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: convert to new logging macros based on struct intel_engine_cs.
On Mon, 13 Jan 2020, Chris Wilson wrote:
> Quoting Wambui Karuga (2020-01-13 11:10:25)
>> fn(...) {
>> ...
>> struct intel_engine_cs *E = ...;
>> +struct drm_i915_private *dev_priv = E->i915;
>
> No new dev_priv.
Wambui, we're gradually converting all dev_priv variable and parameter
names to i915.
> There should be no reason for drm_dbg here, as the rest of the debug is
> behind ENGINE_TRACE and so the vestigial debug should be moved over, or
> deleted as not being useful.
>
> The error messages look unhelpful.
I don't think you can expect any meaninful improvements on the debug
message contents from Wambui without detailed help at this point.
>
>> if ((batch_end - cmd) < length) {
>> - DRM_DEBUG("CMD: Command length exceeds batch length:
>> 0x%08X length=%u batchlen=%td\n",
>> - *cmd,
>> - length,
>> - batch_end - cmd);
>> + drm_dbg(&dev_priv->drm,
>> + "CMD: Command length exceeds batch length:
>> 0x%08X length=%u batchlen=%td\n",
>
> No. This is not driver debug. If anything this should be pr_debug, or
> some over user centric channel.
I'm sorry, I still don't understand your reasoning here.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/panel: simple: fix osd070t1718_19ts sync drive edge
Hi Thierry,
On 02/12/2019 15:07, Laurent Pinchart wrote:
Hi Tomi,
Thank you for the patch.
On Thu, Nov 14, 2019 at 11:39:50AM +0200, Tomi Valkeinen wrote:
The panel datasheet says that the panel samples at falling edge, but
does not say anything about h/v sync signals. Testing shows that if the
sync signals are driven on falling edge, the picture on the panel will
be slightly shifted right.
Setting sync drive edge to the same as data drive edge fixes this issue.
Signed-off-by: Tomi Valkeinen
I don't have access to the documentation, but this makes sense, so
Acked-by: Laurent Pinchart
---
drivers/gpu/drm/panel/panel-simple.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 5d487686d25c..0784536ae6af 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2397,7 +2397,8 @@ static const struct panel_desc
osddisplays_osd070t1718_19ts = {
.height = 91,
},
.bus_format = MEDIA_BUS_FMT_RGB888_1X24,
- .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
+ DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
.connector_type = DRM_MODE_CONNECTOR_DPI,
};
Can you pick this one up?
Tomi
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 6/6] drm/i915: Make WARN* drm specific for various cases.
Drm specific drm_WARN* calls include device information in the
backtrace, so we know what device the warnings originate from.
Covert all the calls of WARN* with device specific drm_WARN*
variants in functions where any one of intel_pm, intel_encoder,
i915_perf_stream or intel_crtc_state struct pointer is available.
The conversion was done automatically with below coccinelle semantic
patch. checkpatch errors/warnings are fixed manually
@@
identifier func, T;
@@
func(struct intel_vgpu *T,...) {
<...
(
-WARN(
+drm_WARN(vgpu_to_dev(T),
...)
|
-WARN_ON(
+drm_WARN_ON(vgpu_to_dev(T),
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(vgpu_to_dev(T),
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(vgpu_to_dev(T),
...)
)
...>
}
@@
identifier func, T;
@@
func(struct intel_encoder *T,...) {
<...
(
-WARN(
+drm_WARN(enc_to_dev(T),
...)
|
-WARN_ON(
+drm_WARN_ON(enc_to_dev(T),
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(enc_to_dev(T),
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(enc_to_dev(T),
...)
)
...>
}
@@
identifier func, T;
@@
func(struct i915_perf_stream *T,...) {
<...
(
-WARN(
+drm_WARN(perf_stream_to_dev(T),
...)
|
-WARN_ON(
+drm_WARN_ON(perf_stream_to_dev(T),
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(perf_stream_to_dev(T),
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(perf_stream_to_dev(T),
...)
)
...>
}
@@
identifier func, T;
@@
func(struct intel_crtc_state *T,...) {
<...
(
-WARN(
+drm_WARN(crtc_state_to_dev(T),
...)
|
-WARN_ON(
+drm_WARN_ON(crtc_state_to_dev(T),
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(crtc_state_to_dev(T),
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(crtc_state_to_dev(T),
...)
)
...>
}
command: spatch --sp-file
[Intel-gfx] [PATCH 5/6] drm/i915: Make WARN* drm specific where dev_priv can be extracted.
Drm specific WARN* calls include device information in the
backtrace, so we know what device the warnings originate from.
Covert all the calls of WARN* with device specific drm_WARN*
variants in functions where first function argument is a
struct pointer and has drm_i915_private struct pointer member.
The conversion was done automatically with below coccinelle semantic
patch. checkpatch errors/warnings are fixed manually.
@r exists@
identifier func, s, i;
position p;
@@
func(struct s *i,...) {
<...
\(WARN@p\|WARN_ON@p\|WARN_ONCE@p\|WARN_ON_ONCE@p\)(...)
...>
}
@rr@
identifier r.s,member;
@@
struct s {
...
struct drm_i915_private *member;
...
};
@rrr@
identifier r.i, rr.member;
position r.p;
@@
(
-WARN@p
+drm_WARN
(
+ i915_to_dev(i->member),
...)
|
-WARN_ON@p
+drm_WARN_ON
(
+ i915_to_dev(i->member),
...)
|
-WARN_ONCE@p
+drm_WARN_ONCE
(
+ i915_to_dev(i->member),
...)
|
-WARN_ON_ONCE@p
+drm_WARN_ON_ONCE
(
+ i915_to_dev(i->member),
...)
)
command: spatch --sp-file
Re: [PATCH 2/3] ARM: dts: am437x-gp/epos-evm: drop unused panel timings
On 12/12/2019 22:35, Laurent Pinchart wrote: Hi Tomi, On Thu, Dec 12, 2019 at 11:37:51AM +0200, Tomi Valkeinen wrote: On 11/12/2019 18:53, Tony Lindgren wrote: * Laurent Pinchart [191202 13:05]: Hi Tomi, Thank you for the patch. On Thu, Nov 14, 2019 at 11:39:49AM +0200, Tomi Valkeinen wrote: panel-simple now handled panel osd070t1718-19ts, and we no longer need the panel timings in the DT file. So remove them. Should you in that case drop the panel-dpi compatible string too, as the panel-dpi bindings require panel timings in DT ? Yeah sounds like if panel-dpi is no longer usable for this device it should be dropped from the compatible list. Ok, I agree. Looking at the dts files, panel-dpi is used in a bunch of boards. But we even have 3 dts files with panel-dpi, without the detailed panel model in compatible... Fixing those will break the compatibility with old dtbs and new kernel, unless we add timings-from-dt to a panel driver that handles panel-dpi. I know, and I don't have a perfect answer for this :-( I don't see a third option, it's either breaking DT backward compatibility or adding timings parsing to a panel driver (either a new panel-dpi driver or to panel-simple). What's your preferred option ? Hmm, I just realized that changing these will break omapfb. It relies on panel-dpi and timings from DT... Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 3/6] drm/i915: Make WARN* drm specific where drm_device ptr available
Drm specific drm_WARN* calls include device information in the
backtrace, so we know what device the warnings originate from.
Covert all the calls of WARN* with device specific drm_WARN*
variants in functions where drm_device struct pointer is readily
available.
The conversion was done automatically with below coccinelle semantic
patch. checkpatch errors/warnings are fixed manually.
@rule1@
identifier func, T;
@@
func(...) {
...
struct drm_device *T = ...;
<...
(
-WARN(
+drm_WARN(T,
...)
|
-WARN_ON(
+drm_WARN_ON(T,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(T,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(T,
...)
)
...>
}
@rule2@
identifier func, T;
@@
func(struct drm_device *T,...) {
<...
(
-WARN(
+drm_WARN(T,
...)
|
-WARN_ON(
+drm_WARN_ON(T,
...)
|
-WARN_ONCE(
+drm_WARN_ONCE(T,
...)
|
-WARN_ON_ONCE(
+drm_WARN_ON_ONCE(T,
...)
)
...>
}
command: spatch --sp-file
[Intel-gfx] [PATCH 2/6] drm/i915: add helper functions to get device ptr
We will need struct drm_device pointer to pass it to drm_WARN* calls.
Add helper functions to exract drm_device pointer from various structs.
Signed-off-by: Pankaj Bharadiya
---
drivers/gpu/drm/i915/display/intel_display_types.h | 14 ++
drivers/gpu/drm/i915/gvt/gvt.h | 5 +
drivers/gpu/drm/i915/i915_drv.h| 11 +++
3 files changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 7964b3dc0046..765878718fe9 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1521,6 +1521,20 @@ dp_to_i915(struct intel_dp *intel_dp)
return to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
}
+static inline struct drm_device *enc_to_dev(const struct intel_encoder
*encoder)
+{
+ return encoder->base.dev;
+}
+
+static inline struct drm_device *
+crtc_state_to_dev(const struct intel_crtc_state *state)
+{
+ struct intel_crtc *crtc = to_intel_crtc(state->uapi.crtc);
+ struct drm_i915_private *i915 = to_i915(crtc->base.dev);
+
+ return i915_to_dev(i915);
+}
+
static inline struct intel_digital_port *
hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
{
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index b47c6acaf9c0..d257399b075a 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -348,6 +348,11 @@ static inline struct intel_gvt *to_gvt(struct
drm_i915_private *i915)
return i915->gvt;
}
+static inline struct drm_device *vgpu_to_dev(const struct intel_vgpu *vgpu)
+{
+ return i915_to_dev(vgpu->gvt->dev_priv);
+}
+
enum {
INTEL_GVT_REQUEST_EMULATE_VBLANK = 0,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e7be4c3e43c6..bdc89d021ff8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1325,6 +1325,17 @@ static inline struct drm_i915_private
*pdev_to_i915(struct pci_dev *pdev)
return pci_get_drvdata(pdev);
}
+static inline struct drm_device *i915_to_dev(struct drm_i915_private *i915)
+{
+ return &i915->drm;
+}
+
+static inline struct drm_device *
+perf_stream_to_dev(const struct i915_perf_stream *stream)
+{
+ return i915_to_dev(stream->perf->i915);
+}
+
/* Simple iterator over all initialised engines */
#define for_each_engine(engine__, dev_priv__, id__) \
for ((id__) = 0; \
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Intel-gfx] [PATCH 0/6]: drm: Introduce struct drm_device based WARN* and use them in i915
Device specific dev_WARN and dev_WARN_ONCE macros available in kernel include device information in the backtrace, so we know what device the warnings originate from. Similar to this, add new struct drm_device based drm_WARN* macros. These macros include device information in the backtrace, so we know what device the warnings originate from. Knowing the device specific information in the backtrace would be helpful in development all around. This patch series aims to convert calls of WARN(), WARN_ON(), WARN_ONCE() and WARN_ON_ONCE() in i916 driver to use the drm device-specific variants automatically wherever struct device pointer is available. To do this, this patch series - - introduces new struct drm_device based WARN* macros - automatically converts WARN* with device specific dev_WARN* variants using coccinelle semantic patch scripts. The goal is to convert all the calls of WARN* with drm_WARN* in i915, but there are still cases where device pointer is not readily available in some functions (or I missed them somehow) using WARN* hence some manual churning is needed. Handle such remaining cases separately later. Changes since RFC at [1] - Introduce drm_WARN* macros and use them as suggested by Sam and Jani - Get rid of extra local variables [1] https://patchwork.freedesktop.org/series/71668/ Pankaj Bharadiya (6): drm/print: introduce new struct drm_device based WARN* macros drm/i915: add helper functions to get device ptr drm/i915: Make WARN* drm specific where drm_device ptr available drm/i915: Make WARN* drm specific where drm_priv ptr is available drm/i915: Make WARN* drm specific where dev_priv can be extracted. drm/i915: Make WARN* drm specific for various cases. drivers/gpu/drm/i915/display/icl_dsi.c| 14 +- drivers/gpu/drm/i915/display/intel_atomic.c | 6 +- drivers/gpu/drm/i915/display/intel_audio.c| 19 +- drivers/gpu/drm/i915/display/intel_bios.c | 10 +- drivers/gpu/drm/i915/display/intel_bw.c | 3 +- drivers/gpu/drm/i915/display/intel_cdclk.c| 81 +++--- drivers/gpu/drm/i915/display/intel_color.c| 3 +- .../gpu/drm/i915/display/intel_combo_phy.c| 2 +- .../gpu/drm/i915/display/intel_connector.c| 3 +- drivers/gpu/drm/i915/display/intel_crt.c | 10 +- drivers/gpu/drm/i915/display/intel_ddi.c | 100 drivers/gpu/drm/i915/display/intel_display.c | 239 ++ .../drm/i915/display/intel_display_power.c| 170 +++-- .../drm/i915/display/intel_display_types.h| 14 + drivers/gpu/drm/i915/display/intel_dp.c | 121 + drivers/gpu/drm/i915/display/intel_dp_mst.c | 10 +- drivers/gpu/drm/i915/display/intel_dpio_phy.c | 3 +- drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 39 +-- drivers/gpu/drm/i915/display/intel_dsb.c | 6 +- .../i915/display/intel_dsi_dcs_backlight.c| 2 +- drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 5 +- drivers/gpu/drm/i915/display/intel_fbc.c | 23 +- drivers/gpu/drm/i915/display/intel_fbdev.c| 13 +- drivers/gpu/drm/i915/display/intel_gmbus.c| 3 +- drivers/gpu/drm/i915/display/intel_hdcp.c | 21 +- drivers/gpu/drm/i915/display/intel_hdmi.c | 71 +++--- drivers/gpu/drm/i915/display/intel_hotplug.c | 7 +- .../gpu/drm/i915/display/intel_lpe_audio.c| 2 +- drivers/gpu/drm/i915/display/intel_lvds.c | 7 +- drivers/gpu/drm/i915/display/intel_opregion.c | 7 +- drivers/gpu/drm/i915/display/intel_overlay.c | 18 +- drivers/gpu/drm/i915/display/intel_panel.c| 21 +- drivers/gpu/drm/i915/display/intel_pipe_crc.c | 7 +- drivers/gpu/drm/i915/display/intel_psr.c | 32 ++- drivers/gpu/drm/i915/display/intel_sdvo.c | 17 +- drivers/gpu/drm/i915/display/intel_sprite.c | 5 +- drivers/gpu/drm/i915/display/intel_tc.c | 17 +- drivers/gpu/drm/i915/display/intel_vdsc.c | 2 +- drivers/gpu/drm/i915/display/vlv_dsi.c| 2 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 4 +- drivers/gpu/drm/i915/gem/i915_gem_pm.c| 4 +- drivers/gpu/drm/i915/gem/i915_gem_shmem.c | 3 +- drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 13 +- drivers/gpu/drm/i915/gem/i915_gem_stolen.c| 15 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 10 +- drivers/gpu/drm/i915/gt/intel_engine_user.c | 3 +- drivers/gpu/drm/i915/gt/intel_gt_irq.c| 15 +- drivers/gpu/drm/i915/gt/intel_gt_pm_irq.c | 2 +- drivers/gpu/drm/i915/gt/intel_gtt.c | 6 +- drivers/gpu/drm/i915/gt/intel_mocs.c | 4 +- drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +- .../gpu/drm/i915/gt/intel_ring_submission.c | 7 +- drivers/gpu/drm/i915/gt/intel_rps.c | 21 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 2 +- drivers/gpu/drm/i915/gvt/aperture_gm.c| 6 +- drivers/gpu/drm/i915/gvt/cfg_space.c | 20 +- drivers/gpu/drm/i915/gvt/cmd_parser.c | 4 +- drivers/gpu/drm/i915/gvt/
[Intel-gfx] [PATCH 1/6] drm/print: introduce new struct drm_device based WARN* macros
Add new struct drm_device based WARN* macros. These are modeled after
the core kernel device based WARN* macros. These would be preferred
over the regular WARN* macros, where possible.
These macros include device information in the backtrace, so we know
what device the warnings originate from.
Knowing the device specific information in the backtrace would be
helpful in development all around.
Signed-off-by: Pankaj Bharadiya
---
include/drm/drm_print.h | 30 ++
1 file changed, 30 insertions(+)
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 8f99d389792d..61a7224e697d 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -553,4 +553,34 @@ void __drm_err(const char *format, ...);
#define DRM_DEBUG_PRIME_RATELIMITED(fmt, ...) \
DRM_DEV_DEBUG_PRIME_RATELIMITED(NULL, fmt, ##__VA_ARGS__)
+/*
+ * struct drm_device based WARNs
+ *
+ *
+ * drm_WARN*() acts like WARN*(), but with the key difference of
+ * using device specific information so that we know from which device
+ * warning is originating from.
+ *
+ * Prefer drm_device based dev_WARN* over regular WARN*
+ */
+
+/* Helper for struct drm_device based WARNs */
+#define drm_WARN(drm, condition, format, arg...) \
+ WARN(condition, "%s %s: " format, \
+ dev_driver_string((drm)->dev), \
+ dev_name((drm)->dev), ## arg)
+
+#define drm_WARN_ONCE(drm, condition, format, arg...) \
+ WARN_ONCE(condition, "%s %s: " format, \
+ dev_driver_string((drm)->dev), \
+ dev_name((drm)->dev), ## arg)
+
+#define drm_WARN_ON(drm, x)\
+ drm_WARN((drm), (x), "%s", \
+"drm_WARN_ON(" __stringify(x) ")")
+
+#define drm_WARN_ON_ONCE(drm, x) \
+ drm_WARN_ONCE((drm), (x), "%s", \
+ "drm_WARN_ON_ONCE(" __stringify(x) ")")
+
#endif /* DRM_PRINT_H_ */
--
2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/i915: convert to new logging macros based on struct intel_engine_cs.
Quoting Wambui Karuga (2020-01-13 11:10:25)
> fn(...) {
> ...
> struct intel_engine_cs *E = ...;
> +struct drm_i915_private *dev_priv = E->i915;
No new dev_priv.
There should be no reason for drm_dbg here, as the rest of the debug is
behind ENGINE_TRACE and so the vestigial debug should be moved over, or
deleted as not being useful.
The error messages look unhelpful.
> if ((batch_end - cmd) < length) {
> - DRM_DEBUG("CMD: Command length exceeds batch length:
> 0x%08X length=%u batchlen=%td\n",
> - *cmd,
> - length,
> - batch_end - cmd);
> + drm_dbg(&dev_priv->drm,
> + "CMD: Command length exceeds batch length:
> 0x%08X length=%u batchlen=%td\n",
No. This is not driver debug. If anything this should be pr_debug, or
some over user centric channel.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] fbdev: potential information leak in do_fb_ioctl()
The "fix" struct has a 2 byte hole after ->ywrapstep and the
"fix = info->fix;" assignment doesn't necessarily clear it. It depends
on the compiler. The solution is just to replace the assignment with an
memcpy().
Fixes: 1f5e31d7e55a ("fbmem: don't call copy_from/to_user() with mutex held")
Signed-off-by: Dan Carpenter
---
v2: Use memcpy()
drivers/video/fbdev/core/fbmem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index d04554959ea7..bb8d8dbc0461 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1115,7 +1115,7 @@ static long do_fb_ioctl(struct fb_info *info, unsigned
int cmd,
break;
case FBIOGET_FSCREENINFO:
lock_fb_info(info);
- fix = info->fix;
+ memcpy(&fix, &info->fix, sizeof(fix));
if (info->flags & FBINFO_HIDE_SMEM_START)
fix.smem_start = 0;
unlock_fb_info(info);
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
DRM driver and runtime suspend-resume handling?
Hi, While working with CRTC color related properties (gamma and CTM for instance) and making them persistent over suspend-resume cycle it occurred to me if I am just wasting resources by storing the property values in the driver and restoring them in dev_pm_ops runtime_resume().. Wouldn't it work if I would just: 1. Add a flag in the driver to indicate that the context may have been lost since the previous atomic commit and set in runtime_resume(). 2. And write the color properties to HW if the context lost flag is set even if the drm_crtc_state color_mgmt_changed is false. The color property values are there despite the color_mgmt_changed == false, aren't they? Best regards, Jyri -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/dp_mst: Have DP_Tx send one msg at a time
[Why]
Noticed this while testing MST with the 4 ports MST hub from
StarTech.com. Sometimes can't light up monitors normally and get the
error message as 'sideband msg build failed'.
Look into aux transactions, found out that source sometimes will send
out another down request before receiving the down reply of the
previous down request. On the other hand, in drm_dp_get_one_sb_msg(),
current code doesn't handle the interleaved replies case. Hence, source
can't build up message completely and can't light up monitors.
[How]
For good compatibility, enforce source to send out one down request at a
time. Add a flag, is_waiting_for_dwn_reply, to determine if the source
can send out a down request immediately or not.
- Check the flag before calling process_single_down_tx_qlock to send out
a msg
- Set the flag when successfully send out a down request
- Clear the flag when successfully build up a down reply
- Clear the flag when find erros during sending out a down request
- Clear the flag when find errors during building up a down reply
- Clear the flag when timeout occurs during waiting for a down reply
- Use drm_dp_mst_kick_tx() to try to send another down request in queue
at the end of drm_dp_mst_wait_tx_reply() (attempt to send out messages
in queue when errors occur)
Cc: Lyude Paul
Signed-off-by: Wayne Lin
---
drivers/gpu/drm/drm_dp_mst_topology.c | 14 --
include/drm/drm_dp_mst_helper.h | 6 ++
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 4395d5cc0645..3542af15387a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1195,6 +1195,8 @@ static int drm_dp_mst_wait_tx_reply(struct
drm_dp_mst_branch *mstb,
txmsg->state == DRM_DP_SIDEBAND_TX_SENT) {
mstb->tx_slots[txmsg->seqno] = NULL;
}
+ mgr->is_waiting_for_dwn_reply = false;
+
}
out:
if (unlikely(ret == -EIO) && drm_debug_enabled(DRM_UT_DP)) {
@@ -1204,6 +1206,7 @@ static int drm_dp_mst_wait_tx_reply(struct
drm_dp_mst_branch *mstb,
}
mutex_unlock(&mgr->qlock);
+ drm_dp_mst_kick_tx(mgr);
return ret;
}
@@ -2770,9 +2773,11 @@ static void process_single_down_tx_qlock(struct
drm_dp_mst_topology_mgr *mgr)
ret = process_single_tx_qlock(mgr, txmsg, false);
if (ret == 1) {
/* txmsg is sent it should be in the slots now */
+ mgr->is_waiting_for_dwn_reply = true;
list_del(&txmsg->next);
} else if (ret) {
DRM_DEBUG_KMS("failed to send msg in q %d\n", ret);
+ mgr->is_waiting_for_dwn_reply = false;
list_del(&txmsg->next);
if (txmsg->seqno != -1)
txmsg->dst->tx_slots[txmsg->seqno] = NULL;
@@ -2812,7 +2817,8 @@ static void drm_dp_queue_down_tx(struct
drm_dp_mst_topology_mgr *mgr,
drm_dp_mst_dump_sideband_msg_tx(&p, txmsg);
}
- if (list_is_singular(&mgr->tx_msg_downq))
+ if (list_is_singular(&mgr->tx_msg_downq) &&
+ !mgr->is_waiting_for_dwn_reply)
process_single_down_tx_qlock(mgr);
mutex_unlock(&mgr->qlock);
}
@@ -3743,6 +3749,7 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
mutex_lock(&mgr->qlock);
txmsg->state = DRM_DP_SIDEBAND_TX_RX;
mstb->tx_slots[slot] = NULL;
+ mgr->is_waiting_for_dwn_reply = false;
mutex_unlock(&mgr->qlock);
wake_up_all(&mgr->tx_waitq);
@@ -3752,6 +3759,9 @@ static int drm_dp_mst_handle_down_rep(struct
drm_dp_mst_topology_mgr *mgr)
no_msg:
drm_dp_mst_topology_put_mstb(mstb);
clear_down_rep_recv:
+ mutex_lock(&mgr->qlock);
+ mgr->is_waiting_for_dwn_reply = false;
+ mutex_unlock(&mgr->qlock);
memset(&mgr->down_rep_recv, 0, sizeof(struct drm_dp_sideband_msg_rx));
return 0;
@@ -4591,7 +4601,7 @@ static void drm_dp_tx_work(struct work_struct *work)
struct drm_dp_mst_topology_mgr *mgr = container_of(work, struct
drm_dp_mst_topology_mgr, tx_work);
mutex_lock(&mgr->qlock);
- if (!list_empty(&mgr->tx_msg_downq))
+ if (!list_empty(&mgr->tx_msg_downq) && !mgr->is_waiting_for_dwn_reply)
process_single_down_tx_qlock(mgr);
mutex_unlock(&mgr->qlock);
}
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 942575de86a0..d0b45468135a 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -610,6 +610,12 @@ struct drm_dp_mst_topology_mgr {
* &drm_dp_sideband_msg_tx.state once they are queued
*/
struct mutex qlock;
+
+ /**
+* @is_waiting_for_dwn_reply: indicate whether is waiting for down reply
+*/
+ bool is_waiting_for_dwn_reply;
+
/**
Re: [PATCH] drm/cirrus: Let DRM core send VBLANK events
Hi
Am 13.01.20 um 00:00 schrieb Daniel Vetter:
> On Fri, Jan 10, 2020 at 12:57:07PM +0100, Thomas Zimmermann wrote:
>> In drm_atomic_helper_fake_vblank() the DRM core sends out VBLANK
>> events if struct drm_crtc_state.no_vblank is enabled. Replace cirrus'
>> VBLANK events with the DRM core's functionality.
>>
>> Signed-off-by: Thomas Zimmermann
>> ---
>> drivers/gpu/drm/cirrus/cirrus.c | 10 ++
>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/cirrus/cirrus.c
>> b/drivers/gpu/drm/cirrus/cirrus.c
>> index 248c9f765c45..4a1729aa7e53 100644
>> --- a/drivers/gpu/drm/cirrus/cirrus.c
>> +++ b/drivers/gpu/drm/cirrus/cirrus.c
>> @@ -38,7 +38,6 @@
>> #include
>> #include
>> #include
>> -#include
>>
>> #define DRIVER_NAME "cirrus"
>> #define DRIVER_DESC "qemu cirrus vga"
>> @@ -415,6 +414,8 @@ static void cirrus_pipe_enable(struct
>> drm_simple_display_pipe *pipe,
>> {
>> struct cirrus_device *cirrus = pipe->crtc.dev->dev_private;
>>
>> +crtc_state->no_vblank = true;
>
> Huh, nice untended use of this stuff ... We've added this for writeback,
> but I guess it can be used for anything that's a virtual connector ...
Oh, 'improved by accident'.
I'm not quite sure what you mean by virtual connector, but it should
work with any CRTC without VBLANK support. At least I've never seen any
problem with ast and udl. I'll update the docs accordingly.
Best regards
Thomas
>
> I've also spotted that you've done this same trick for ast & udl already.
> But I think before we roll this out massively we should make this
> official. Can you pls do a patch to update the kerneldoc for @no_vblank
> that virtual hw can also use this stuff?
>
> Also, computing state values in atomic_commit code is kinda uncool and
> fraught with peril - design assumption is that with some very few
> exceptions (which are kinda awkward, would be nice to make state pointers
> const) all the core and helper codes that state structures stay unchanged
> after atomic_check completed. This should be computed in atomic_check (like
> vc4
> does). Can you pls also include patches to update ast and udl in this
> series?
>
> Thanks, Daniel
>
>
>> +
>> cirrus_mode_set(cirrus, &crtc_state->mode, plane_state->fb);
>> cirrus_fb_blit_fullscreen(plane_state->fb);
>> }
>> @@ -434,13 +435,6 @@ static void cirrus_pipe_update(struct
>> drm_simple_display_pipe *pipe,
>>
>> if (drm_atomic_helper_damage_merged(old_state, state, &rect))
>> cirrus_fb_blit_rect(pipe->plane.state->fb, &rect);
>> -
>> -if (crtc->state->event) {
>> -spin_lock_irq(&crtc->dev->event_lock);
>> -drm_crtc_send_vblank_event(crtc, crtc->state->event);
>> -crtc->state->event = NULL;
>> -spin_unlock_irq(&crtc->dev->event_lock);
>> -}
>> }
>>
>> static const struct drm_simple_display_pipe_funcs cirrus_pipe_funcs = {
>> --
>> 2.24.1
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
signature.asc
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 206155] amdgpu several warnings while booting Fiji GPU, GPU not activated
https://bugzilla.kernel.org/show_bug.cgi?id=206155 --- Comment #3 from Janpieter Sollie (janpieter.sol...@dommel.be) --- Created attachment 286777 --> https://bugzilla.kernel.org/attachment.cgi?id=286777&action=edit amdgpu module load with all debugging turned on hereby the debug info of the amdgpu module load. Whereas I could trace the stack, this looks kinda Chinese to me, but I hope someone is able to find a value that does not belong there. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Warnings in DRM code when removing/unbinding a driver
Hi John Am 10.01.20 um 13:54 schrieb John Garry: > > > Hi Thomas, > >> drm-tip now contains > > I have tested today's linux-next, which includes this: > >> >> commit a88248506a2bcfeaef6837a53cde19fe11970e6c >> Author: Thomas Zimmermann >> Date: Tue Dec 3 09:38:15 2019 +0100 >> >> drm/hisilicon/hibmc: Switch to generic fbdev emulation >> >> which removes this entire code and switches hibmc to generic fbdev >> emulation. Does that fix the problem? >> > > And I see no warn, here's a dmesg snippet: Great. So I'll consider this fixed. Thanks for reporting ad testing. Best regards Thomas > > [ 20.672787] pci 0007:90:00.0: can't derive routing for PCI INT A > [ 20.678831] hibmc-drm 0007:91:00.0: PCI INT A: no GSI > [ 20.686536] pci_bus 0007:90: 2-byte config write to 0007:90:00.0 > offset 0x4 may corrupt adjacent RW1C bits > [ 20.696888] [TTM] Zone kernel: Available graphics memory: 57359458 KiB > [ 20.703545] [TTM] Zone dma32: Available graphics memory: 2097152 KiB > [ 20.710108] [TTM] Initializing pool allocator > [ 20.714561] [TTM] Initializing DMA pool allocator > [ 20.720212] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [ 20.726863] [drm] No driver support for vblank timestamp query. > [ 20.754777] Console: switching to colour frame buffer device 100x37 > [ 20.778180] hibmc-drm 0007:91:00.0: fb0: hibmcdrmfb frame buffer device > [ 20.786447] [drm] Initialized hibmc 1.0.0 20160828 for 0007:91:00.0 > on minor 0 > [ 20.794346] Console: switching to colour dummy device 80x25 > [ 20.801884] pci 0007:90:00.0: can't derive routing for PCI INT A > [ 20.807963] hibmc-drm 0007:91:00.0: PCI INT A: no GSI > [ 20.813656] [TTM] Finalizing pool allocator > [ 20.817905] [TTM] Finalizing DMA pool allocator > [ 20.822576] [TTM] Zone kernel: Used memory at exit: 0 KiB > [ 20.828760] [TTM] Zone dma32: Used memory at exit: 0 KiB > [ 20.834978] pci 0007:90:00.0: can't derive routing for PCI INT A > [ 20.841021] hibmc-drm 0007:91:00.0: PCI INT A: no GSI > [ 20.848858] [TTM] Zone kernel: Available graphics memory: 57359458 KiB > [ 20.855516] [TTM] Zone dma32: Available graphics memory: 2097152 KiB > [ 20.862079] [TTM] Initializing pool allocator > [ 20.866525] [TTM] Initializing DMA pool allocator > [ 20.872064] [drm] Supports vblank timestamp caching Rev 2 (21.10.2013). > [ 20.878716] [drm] No driver support for vblank timestamp query. > [ 20.905996] Console: switching to colour frame buffer device 100x37 > [ 20.929385] hibmc-drm 0007:91:00.0: fb0: hibmcdrmfb frame buffer device > [ 20.937241] [drm] Initialized hibmc 1.0.0 20160828 for 0007:91:00.0 > on minor 0 > [ 21.171906] loop: module loaded > > Thanks, > John > >> Best regards >> Thomas >> >>> [ 27.965802] hibmc_unload+0x2c/0xd0 >>> [ 27.969281] hibmc_pci_remove+0x2c/0x40 >>> [ 27.973109] pci_device_remove+0x6c/0x140 >>> [ 27.977110] really_probe+0x174/0x548 >>> [ 27.980763] driver_probe_device+0x7c/0x148 >>> [ 27.984936] device_driver_attach+0x94/0xa0 >>> [ 27.989109] __driver_attach+0xa8/0x110 >>> [ 27.992935] bus_for_each_dev+0xe8/0x158 >>> [ 27.996849] driver_attach+0x30/0x40 >>> [ 28.000415] bus_add_driver+0x234/0x2f0 >>> [ 28.004241] driver_register+0xbc/0x1d0 >>> [ 28.008067] __pci_register_driver+0xbc/0xd0 >>> [ 28.012329] hibmc_pci_driver_init+0x20/0x28 >>> [ 28.016590] do_one_initcall+0xb4/0x254 >>> [ 28.020417] kernel_init_freeable+0x27c/0x328 >>> [ 28.024765] kernel_init+0x10/0x118 >>> [ 28.028245] ret_from_fork+0x10/0x18 >>> [ 28.031813] ---[ end trace 35a83b71b657878d ]--- >>> [ 28.036503] [ cut here ] >>> [ 28.041115] WARNING: CPU: 24 PID: 1 at >>> drivers/gpu/drm/drm_gem_vram_helper.c:40 >>> ttm_buffer_object_destroy+0x4c/0x80 >>> [ 28.051537] Modules linked in: >>> [ 28.054585] CPU: 24 PID: 1 Comm: swapper/0 Tainted: G B W >>> 5.5.0-rc1-dirty #565 >>> [ 28.062924] Hardware name: Huawei D06 /D06, BIOS Hisilicon D06 UEFI >>> RC0 - V1.16.01 03/15/2019 >>> >>> [snip] >>> >>> Indeed, simply unbinding the device from the driver causes the same sort >>> of issue: >>> >>> root@(none)$ cd ./bus/pci/drivers/hibmc-drm/ >>> root@(none)$ ls >>> :05:00.0 bind new_id remove_id uevent >>> unbind >>> root@(none)$ echo \:05\:00.0 > unbind >>> [ 116.074352] [ cut here ] >>> [ 116.078978] WARNING: CPU: 17 PID: 1178 at >>> drivers/gpu/drm/drm_gem_vram_helper.c:40 >>> ttm_buffer_object_destroy+0x4c/0x80 >>> [ 116.089661] Modules linked in: >>> [ 116.092711] CPU: 17 PID: 1178 Comm: sh Tainted: G B W >>> 5.5.0-rc1-dirty #565 >>> [ 116.100704] Hardware name: Huawei D06 /D06, BIOS Hisilicon D06 UEFI >>> RC0 - V1.16.01 03/15/2019 >>> [ 116.109218] pstate: 2049 (nzCv daif +PAN -UAO) >>> [ 116.114001] pc : ttm_buffer_object_destroy+0x4c/0x80 >>> [ 116.118956] lr : ttm_buffer_object_destroy+0x18/0x80 >>> [ 116.123910] s
