| Message ID | 1443638858-16895-1-git-send-email-alexander.deucher@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30.09.2015 20:47, Alex Deucher wrote: > If a driver uses the cursor_set2 crtc callback rather than > cursor_set, use that. This fixes the fbdev helper for drivers > that use cursor_set2. > > Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> I'm not very familiar with the code, but that looks like it makes sense. Both patches are Reviewed-by: Christian König <christian.koenig@amd.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 418d299..ca08c47 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) > struct drm_crtc *crtc = mode_set->crtc; > int ret; > > - if (crtc->funcs->cursor_set) { > + if (crtc->funcs->cursor_set2) { > + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); > + if (ret) > + error = true; > + } else if (crtc->funcs->cursor_set) { > ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); > if (ret) > error = true;
On 01.10.2015 03:47, Alex Deucher wrote: > If a driver uses the cursor_set2 crtc callback rather than > cursor_set, use that. This fixes the fbdev helper for drivers > that use cursor_set2. > > Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 418d299..ca08c47 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) > struct drm_crtc *crtc = mode_set->crtc; > int ret; > > - if (crtc->funcs->cursor_set) { > + if (crtc->funcs->cursor_set2) { > + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); > + if (ret) > + error = true; > + } else if (crtc->funcs->cursor_set) { > ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); > if (ret) > error = true; > Hah, nice catch. The series is Reviewed-by: Michel Dänzer <michel.daenzer@amd.com> (I assume you tested with something like killall -9 Xorg and confirming sure the cursor doesn't stay visible in console)
Hi Alex, On 30 September 2015 at 19:47, Alex Deucher <alexdeucher@gmail.com> wrote: > If a driver uses the cursor_set2 crtc callback rather than > cursor_set, use that. This fixes the fbdev helper for drivers > that use cursor_set2. > > Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 Are Change-Id lines accepted in the kernel ? Iirc there was some noise against them a while back. Thanks Emil
On Thu, Oct 01, 2015 at 12:36:27PM +0100, Emil Velikov wrote: > Hi Alex, > > On 30 September 2015 at 19:47, Alex Deucher <alexdeucher@gmail.com> wrote: > > If a driver uses the cursor_set2 crtc callback rather than > > cursor_set, use that. This fixes the fbdev helper for drivers > > that use cursor_set2. > > > > Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 > Are Change-Id lines accepted in the kernel ? Iirc there was some noise > against them a while back. Some maintainers reject them, I don't think anyone here in the drm subsystem cares really. -Daniel
> -----Original Message----- > From: Emil Velikov [mailto:emil.l.velikov@gmail.com] > Sent: Thursday, October 01, 2015 7:36 AM > To: Alex Deucher > Cc: ML dri-devel; Deucher, Alexander > Subject: Re: [PATCH 1/2] drm: handle cursor_set2 in restore_fbdev_mode > > Hi Alex, > > On 30 September 2015 at 19:47, Alex Deucher <alexdeucher@gmail.com> > wrote: > > If a driver uses the cursor_set2 crtc callback rather than > > cursor_set, use that. This fixes the fbdev helper for drivers > > that use cursor_set2. > > > > Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 > Are Change-Id lines accepted in the kernel ? Iirc there was some noise > against them a while back. It was just a temporary problem with my git client. I've already fixed it locally. Alex > > Thanks > Emil
On Wed, Sep 30, 2015 at 11:50 PM, Michel Dänzer <michel@daenzer.net> wrote: > On 01.10.2015 03:47, Alex Deucher wrote: >> If a driver uses the cursor_set2 crtc callback rather than >> cursor_set, use that. This fixes the fbdev helper for drivers >> that use cursor_set2. >> >> Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 >> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >> --- >> drivers/gpu/drm/drm_fb_helper.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index 418d299..ca08c47 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >> struct drm_crtc *crtc = mode_set->crtc; >> int ret; >> >> - if (crtc->funcs->cursor_set) { >> + if (crtc->funcs->cursor_set2) { >> + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); >> + if (ret) >> + error = true; >> + } else if (crtc->funcs->cursor_set) { >> ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); >> if (ret) >> error = true; >> > > Hah, nice catch. The series is > > Reviewed-by: Michel Dänzer <michel.daenzer@amd.com> > > (I assume you tested with something like killall -9 Xorg and confirming > sure the cursor doesn't stay visible in console) > The cursor still stays visible in the console. Interestingly, the cursor still shows up in the console even without these patches. Alex
On Thu, Oct 1, 2015 at 1:01 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Wed, Sep 30, 2015 at 11:50 PM, Michel Dänzer <michel@daenzer.net> wrote: >> On 01.10.2015 03:47, Alex Deucher wrote: >>> If a driver uses the cursor_set2 crtc callback rather than >>> cursor_set, use that. This fixes the fbdev helper for drivers >>> that use cursor_set2. >>> >>> Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 >>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >>> --- >>> drivers/gpu/drm/drm_fb_helper.c | 6 +++++- >>> 1 file changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >>> index 418d299..ca08c47 100644 >>> --- a/drivers/gpu/drm/drm_fb_helper.c >>> +++ b/drivers/gpu/drm/drm_fb_helper.c >>> @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >>> struct drm_crtc *crtc = mode_set->crtc; >>> int ret; >>> >>> - if (crtc->funcs->cursor_set) { >>> + if (crtc->funcs->cursor_set2) { >>> + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); >>> + if (ret) >>> + error = true; >>> + } else if (crtc->funcs->cursor_set) { >>> ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); >>> if (ret) >>> error = true; >>> >> >> Hah, nice catch. The series is >> >> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com> >> >> (I assume you tested with something like killall -9 Xorg and confirming >> sure the cursor doesn't stay visible in console) >> > > The cursor still stays visible in the console. Interestingly, the > cursor still shows up in the console even without these patches. restore_fbdev_mode doesn't end up get called when I killall -9 X. Alex > > Alex
On Thu, Oct 1, 2015 at 1:13 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > On Thu, Oct 1, 2015 at 1:01 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Wed, Sep 30, 2015 at 11:50 PM, Michel Dänzer <michel@daenzer.net> wrote: >>> On 01.10.2015 03:47, Alex Deucher wrote: >>>> If a driver uses the cursor_set2 crtc callback rather than >>>> cursor_set, use that. This fixes the fbdev helper for drivers >>>> that use cursor_set2. >>>> >>>> Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 >>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >>>> --- >>>> drivers/gpu/drm/drm_fb_helper.c | 6 +++++- >>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >>>> index 418d299..ca08c47 100644 >>>> --- a/drivers/gpu/drm/drm_fb_helper.c >>>> +++ b/drivers/gpu/drm/drm_fb_helper.c >>>> @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >>>> struct drm_crtc *crtc = mode_set->crtc; >>>> int ret; >>>> >>>> - if (crtc->funcs->cursor_set) { >>>> + if (crtc->funcs->cursor_set2) { >>>> + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); >>>> + if (ret) >>>> + error = true; >>>> + } else if (crtc->funcs->cursor_set) { >>>> ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); >>>> if (ret) >>>> error = true; >>>> >>> >>> Hah, nice catch. The series is >>> >>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com> >>> >>> (I assume you tested with something like killall -9 Xorg and confirming >>> sure the cursor doesn't stay visible in console) >>> >> >> The cursor still stays visible in the console. Interestingly, the >> cursor still shows up in the console even without these patches. > > restore_fbdev_mode doesn't end up get called when I killall -9 X. For clarity, drm_fb_helper_set_par which which ultimately calls restore_fbdev_mode never gets called. Alex > > Alex > >> >> Alex
On Thu, Oct 01, 2015 at 01:22:42PM -0400, Alex Deucher wrote: > On Thu, Oct 1, 2015 at 1:13 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > > On Thu, Oct 1, 2015 at 1:01 PM, Alex Deucher <alexdeucher@gmail.com> wrote: > >> On Wed, Sep 30, 2015 at 11:50 PM, Michel Dänzer <michel@daenzer.net> wrote: > >>> On 01.10.2015 03:47, Alex Deucher wrote: > >>>> If a driver uses the cursor_set2 crtc callback rather than > >>>> cursor_set, use that. This fixes the fbdev helper for drivers > >>>> that use cursor_set2. > >>>> > >>>> Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 > >>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > >>>> --- > >>>> drivers/gpu/drm/drm_fb_helper.c | 6 +++++- > >>>> 1 file changed, 5 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > >>>> index 418d299..ca08c47 100644 > >>>> --- a/drivers/gpu/drm/drm_fb_helper.c > >>>> +++ b/drivers/gpu/drm/drm_fb_helper.c > >>>> @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) > >>>> struct drm_crtc *crtc = mode_set->crtc; > >>>> int ret; > >>>> > >>>> - if (crtc->funcs->cursor_set) { > >>>> + if (crtc->funcs->cursor_set2) { > >>>> + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); > >>>> + if (ret) > >>>> + error = true; > >>>> + } else if (crtc->funcs->cursor_set) { > >>>> ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); > >>>> if (ret) > >>>> error = true; > >>>> > >>> > >>> Hah, nice catch. The series is > >>> > >>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com> > >>> > >>> (I assume you tested with something like killall -9 Xorg and confirming > >>> sure the cursor doesn't stay visible in console) > >>> > >> > >> The cursor still stays visible in the console. Interestingly, the > >> cursor still shows up in the console even without these patches. > > > > restore_fbdev_mode doesn't end up get called when I killall -9 X. > > For clarity, drm_fb_helper_set_par which which ultimately calls > restore_fbdev_mode never gets called. You need to force restore the fbdev in your lastclose hook for that to work. -Daniel
On 02.10.2015 02:22, Alex Deucher wrote: > On Thu, Oct 1, 2015 at 1:13 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >> On Thu, Oct 1, 2015 at 1:01 PM, Alex Deucher <alexdeucher@gmail.com> wrote: >>> On Wed, Sep 30, 2015 at 11:50 PM, Michel Dänzer <michel@daenzer.net> wrote: >>>> On 01.10.2015 03:47, Alex Deucher wrote: >>>>> If a driver uses the cursor_set2 crtc callback rather than >>>>> cursor_set, use that. This fixes the fbdev helper for drivers >>>>> that use cursor_set2. >>>>> >>>>> Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 >>>>> Signed-off-by: Alex Deucher <alexander.deucher@amd.com> >>>>> --- >>>>> drivers/gpu/drm/drm_fb_helper.c | 6 +++++- >>>>> 1 file changed, 5 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >>>>> index 418d299..ca08c47 100644 >>>>> --- a/drivers/gpu/drm/drm_fb_helper.c >>>>> +++ b/drivers/gpu/drm/drm_fb_helper.c >>>>> @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) >>>>> struct drm_crtc *crtc = mode_set->crtc; >>>>> int ret; >>>>> >>>>> - if (crtc->funcs->cursor_set) { >>>>> + if (crtc->funcs->cursor_set2) { >>>>> + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); >>>>> + if (ret) >>>>> + error = true; >>>>> + } else if (crtc->funcs->cursor_set) { >>>>> ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); >>>>> if (ret) >>>>> error = true; >>>>> >>>> >>>> Hah, nice catch. The series is >>>> >>>> Reviewed-by: Michel Dänzer <michel.daenzer@amd.com> >>>> >>>> (I assume you tested with something like killall -9 Xorg and confirming >>>> sure the cursor doesn't stay visible in console) >>>> >>> >>> The cursor still stays visible in the console. Interestingly, the >>> cursor still shows up in the console even without these patches. >> >> restore_fbdev_mode doesn't end up get called when I killall -9 X. > > For clarity, drm_fb_helper_set_par which which ultimately calls > restore_fbdev_mode never gets called. Hmm, radeon_fb_helper_set_par definitely got hit when I was adding it, but maybe I was only testing with killall -3 or something like that.
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 418d299..ca08c47 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -345,7 +345,11 @@ static bool restore_fbdev_mode(struct drm_fb_helper *fb_helper) struct drm_crtc *crtc = mode_set->crtc; int ret; - if (crtc->funcs->cursor_set) { + if (crtc->funcs->cursor_set2) { + ret = crtc->funcs->cursor_set2(crtc, NULL, 0, 0, 0, 0, 0); + if (ret) + error = true; + } else if (crtc->funcs->cursor_set) { ret = crtc->funcs->cursor_set(crtc, NULL, 0, 0, 0); if (ret) error = true;
If a driver uses the cursor_set2 crtc callback rather than cursor_set, use that. This fixes the fbdev helper for drivers that use cursor_set2. Change-Id: I127d3f8e72789ba70c3648140f87c6187864e130 Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/drm_fb_helper.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)