| Message ID | 66f8959522a679d80eb71ba8dae47b86d94e71d9.1722263308.git.jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/xe & drm/i915: drvdata usage changes | expand |
Quoting Jani Nikula (2024-07-29 11:30:04-03:00) >The test code seems to assume struct drm_device * is stored in >drvdata. This is (currently) not the case. Use the proper helper to get >at the xe device. > >This has not been an issue, because struct drm_device is embedded in >struct xe_device at offset 0. > >Signed-off-by: Jani Nikula <jani.nikula@intel.com> The fix looks correct, so: Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> I noticed that xe_call_for_each_device() stopped being used as of commit 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could also have a patch removing it and dev_to_xe_device_fn(). -- Gustavo Sousa >--- > drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > >diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c >index 577ee7d14381..2046789f62bd 100644 >--- a/drivers/gpu/drm/xe/tests/xe_pci.c >+++ b/drivers/gpu/drm/xe/tests/xe_pci.c >@@ -20,15 +20,15 @@ struct kunit_test_data { > static int dev_to_xe_device_fn(struct device *dev, void *__data) > > { >- struct drm_device *drm = dev_get_drvdata(dev); >+ struct xe_device *xe = kdev_to_xe_device(dev); > struct kunit_test_data *data = __data; > int ret = 0; > int idx; > > data->ndevs++; > >- if (drm_dev_enter(drm, &idx)) >- ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev))); >+ if (drm_dev_enter(&xe->drm, &idx)) >+ ret = data->xe_fn(xe); > drm_dev_exit(idx); > > return ret; >-- >2.39.2 >
On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: > Quoting Jani Nikula (2024-07-29 11:30:04-03:00) >>The test code seems to assume struct drm_device * is stored in >>drvdata. This is (currently) not the case. Use the proper helper to get >>at the xe device. >> >>This has not been an issue, because struct drm_device is embedded in >>struct xe_device at offset 0. >> >>Signed-off-by: Jani Nikula <jani.nikula@intel.com> > > The fix looks correct, so: > > Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> > > I noticed that xe_call_for_each_device() stopped being used as of commit > 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could > also have a patch removing it and dev_to_xe_device_fn(). Cc: people involved with that commit. Do you want xe_call_for_each_device() removed or retained? BR, Jani. > > -- > Gustavo Sousa > >>--- >> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c >>index 577ee7d14381..2046789f62bd 100644 >>--- a/drivers/gpu/drm/xe/tests/xe_pci.c >>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c >>@@ -20,15 +20,15 @@ struct kunit_test_data { >> static int dev_to_xe_device_fn(struct device *dev, void *__data) >> >> { >>- struct drm_device *drm = dev_get_drvdata(dev); >>+ struct xe_device *xe = kdev_to_xe_device(dev); >> struct kunit_test_data *data = __data; >> int ret = 0; >> int idx; >> >> data->ndevs++; >> >>- if (drm_dev_enter(drm, &idx)) >>- ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev))); >>+ if (drm_dev_enter(&xe->drm, &idx)) >>+ ret = data->xe_fn(xe); >> drm_dev_exit(idx); >> >> return ret; >>-- >>2.39.2 >>
On Tue, 06 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote: > On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: >> Quoting Jani Nikula (2024-07-29 11:30:04-03:00) >>>The test code seems to assume struct drm_device * is stored in >>>drvdata. This is (currently) not the case. Use the proper helper to get >>>at the xe device. >>> >>>This has not been an issue, because struct drm_device is embedded in >>>struct xe_device at offset 0. >>> >>>Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> >> The fix looks correct, so: >> >> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> >> >> I noticed that xe_call_for_each_device() stopped being used as of commit >> 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could >> also have a patch removing it and dev_to_xe_device_fn(). > > Cc: people involved with that commit. And now actually Cc. *facepalm* > > Do you want xe_call_for_each_device() removed or retained? > > BR, > Jani. > > >> >> -- >> Gustavo Sousa >> >>>--- >>> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c >>>index 577ee7d14381..2046789f62bd 100644 >>>--- a/drivers/gpu/drm/xe/tests/xe_pci.c >>>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c >>>@@ -20,15 +20,15 @@ struct kunit_test_data { >>> static int dev_to_xe_device_fn(struct device *dev, void *__data) >>> >>> { >>>- struct drm_device *drm = dev_get_drvdata(dev); >>>+ struct xe_device *xe = kdev_to_xe_device(dev); >>> struct kunit_test_data *data = __data; >>> int ret = 0; >>> int idx; >>> >>> data->ndevs++; >>> >>>- if (drm_dev_enter(drm, &idx)) >>>- ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev))); >>>+ if (drm_dev_enter(&xe->drm, &idx)) >>>+ ret = data->xe_fn(xe); >>> drm_dev_exit(idx); >>> >>> return ret; >>>-- >>>2.39.2 >>>
On Tue, Aug 06, 2024 at 03:14:02PM GMT, Jani Nikula wrote: >On Tue, 06 Aug 2024, Jani Nikula <jani.nikula@intel.com> wrote: >> On Thu, 01 Aug 2024, Gustavo Sousa <gustavo.sousa@intel.com> wrote: >>> Quoting Jani Nikula (2024-07-29 11:30:04-03:00) >>>>The test code seems to assume struct drm_device * is stored in >>>>drvdata. This is (currently) not the case. Use the proper helper to get >>>>at the xe device. >>>> >>>>This has not been an issue, because struct drm_device is embedded in >>>>struct xe_device at offset 0. >>>> >>>>Signed-off-by: Jani Nikula <jani.nikula@intel.com> >>> >>> The fix looks correct, so: >>> >>> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com> >>> >>> I noticed that xe_call_for_each_device() stopped being used as of commit >>> 57ecead343e7 ("drm/xe/tests: Convert xe_mocs live tests"), so we could >>> also have a patch removing it and dev_to_xe_device_fn(). >> >> Cc: people involved with that commit. > >And now actually Cc. *facepalm* > >> >> Do you want xe_call_for_each_device() removed or retained? removed... it should had been removed as part of that series as it only reflects the previous approach. My bad for not catching it during review. struct kunit_test_data and dev_to_xe_device_fn() should be gone too. Lucas De Marchi >> >> BR, >> Jani. >> >> >>> >>> -- >>> Gustavo Sousa >>> >>>>--- >>>> drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>>diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c >>>>index 577ee7d14381..2046789f62bd 100644 >>>>--- a/drivers/gpu/drm/xe/tests/xe_pci.c >>>>+++ b/drivers/gpu/drm/xe/tests/xe_pci.c >>>>@@ -20,15 +20,15 @@ struct kunit_test_data { >>>> static int dev_to_xe_device_fn(struct device *dev, void *__data) >>>> >>>> { >>>>- struct drm_device *drm = dev_get_drvdata(dev); >>>>+ struct xe_device *xe = kdev_to_xe_device(dev); >>>> struct kunit_test_data *data = __data; >>>> int ret = 0; >>>> int idx; >>>> >>>> data->ndevs++; >>>> >>>>- if (drm_dev_enter(drm, &idx)) >>>>- ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev))); >>>>+ if (drm_dev_enter(&xe->drm, &idx)) >>>>+ ret = data->xe_fn(xe); >>>> drm_dev_exit(idx); >>>> >>>> return ret; >>>>-- >>>>2.39.2 >>>> > >-- >Jani Nikula, Intel
diff --git a/drivers/gpu/drm/xe/tests/xe_pci.c b/drivers/gpu/drm/xe/tests/xe_pci.c index 577ee7d14381..2046789f62bd 100644 --- a/drivers/gpu/drm/xe/tests/xe_pci.c +++ b/drivers/gpu/drm/xe/tests/xe_pci.c @@ -20,15 +20,15 @@ struct kunit_test_data { static int dev_to_xe_device_fn(struct device *dev, void *__data) { - struct drm_device *drm = dev_get_drvdata(dev); + struct xe_device *xe = kdev_to_xe_device(dev); struct kunit_test_data *data = __data; int ret = 0; int idx; data->ndevs++; - if (drm_dev_enter(drm, &idx)) - ret = data->xe_fn(to_xe_device(dev_get_drvdata(dev))); + if (drm_dev_enter(&xe->drm, &idx)) + ret = data->xe_fn(xe); drm_dev_exit(idx); return ret;
The test code seems to assume struct drm_device * is stored in drvdata. This is (currently) not the case. Use the proper helper to get at the xe device. This has not been an issue, because struct drm_device is embedded in struct xe_device at offset 0. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/xe/tests/xe_pci.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)