On Mon, Oct 02, 2023 at 07:58:30PM +0300, Jani Nikula wrote: > On Mon, 02 Oct 2023, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote: > > On Mon, Oct 02, 2023 at 10:41:14AM +0300, Jani Nikula wrote: > >> On Fri, 29 Sep 2023, Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> wrote: > >> > The goal is to have this function ready for Xe to use > >> > directly. So, let's use the available macro. > >> > >> Seesm wrong to use DISPLAY_INFO() as an lvalue > > > > to be really honestly I don't like that either. > > I barely like macros, specially used like this. > > > >> and I'm not sure why > >> this wouldn't work as-is. > > > > I should probably had collected some logs and added to the > > commit message. But the thing was that without this assignment, > > (xe)->info.display was NULL and the memcpy below was exploding > > with NULL dereference. > > Aww crap. That's because both DISPLAY_INFO() and DISPLAY_RUNTIME_INFO() > in xe are completely bogus. > > They should be > > #define DISPLAY_INFO(i915) ((i915)->display.info.__device_info) > #define DISPLAY_RUNTIME_INFO(i915) (&(i915)->display.info.__runtime_info) > > instead of > > #define DISPLAY_INFO(xe) ((xe)->info.display) > #define DISPLAY_RUNTIME_INFO(xe) (&(xe)->info.display_runtime) > > and these should be removed from struct xe_device info member: > > const struct intel_display_device_info *display; > struct intel_display_runtime_info display_runtime; but in this case we would need the macros in Xe to resolve the access to these items anyway right?! or how should we handle cases like 'if (xe->info.display_runtime.pipe_mask)' ? > > BR, > Jani. > > > > > >> > >> But *shrug*. > >> > >> Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx> > > > > thanks, pushed as is. > > > >> > >> for merging to i915. (xe should come as a backport with cherry-pick -x.) > > > > and sent the proper backported cherry-pick to intel-xe ml. > > > >> > >> BR, > >> Jani > >> > >> > >> > > >> > Cc: Jani Nikula <jani.nikula@xxxxxxxxx> > >> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > >> > --- > >> > drivers/gpu/drm/i915/display/intel_display_device.c | 2 +- > >> > 1 file changed, 1 insertion(+), 1 deletion(-) > >> > > >> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c > >> > index a6a18eae7ae8..ce55b968e658 100644 > >> > --- a/drivers/gpu/drm/i915/display/intel_display_device.c > >> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.c > >> > @@ -926,7 +926,7 @@ void intel_display_device_probe(struct drm_i915_private *i915) > >> > else > >> > info = probe_display(i915); > >> > > >> > - i915->display.info.__device_info = info; > >> > + DISPLAY_INFO(i915) = info; > >> > > >> > memcpy(DISPLAY_RUNTIME_INFO(i915), > >> > &DISPLAY_INFO(i915)->__runtime_defaults, > >> > >> -- > >> Jani Nikula, Intel > > -- > Jani Nikula, Intel
