diff mbox series

[08/14] drm/amdgpu: Remove default best_encoder hook from DC

Message ID 20180903165439.24845-8-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series [01/14] drm: Add drm/kernel.h header file | expand

Commit Message

Daniel Vetter Sept. 3, 2018, 4:54 p.m. UTC
For atomic driver this is the default, no need to reimplement it. We
still need to keep the copypasta for not-atomic drivers though, since
no one polished the legacy crtc helpers as much as the atomic ones.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Harry Wentland <harry.wentland@amd.com>
Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
Cc: Tony Cheng <Tony.Cheng@amd.com>
Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
Cc: Shirish S <shirish.s@amd.com>
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 -------------------
 1 file changed, 23 deletions(-)

Comments

Leo Sept. 5, 2018, 1:45 p.m. UTC | #1
On 2018-09-03 12:54 PM, Daniel Vetter wrote:
> For atomic driver this is the default, no need to reimplement it. We
> still need to keep the copypasta for not-atomic drivers though, since
> no one polished the legacy crtc helpers as much as the atomic ones.

Thanks for the patch! It seems I made an identical change here:
https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html

One line difference though. I wasn't aware that the default is used when
.best_encoder is null, so I just hooked it to the default helper.

If it's OK with you, I'll do a follow-up to drop the hook, so we don't
have two conflicting patches in flight.

Leo

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Harry Wentland <harry.wentland@amd.com>
> Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> Cc: Tony Cheng <Tony.Cheng@amd.com>
> Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
> Cc: Shirish S <shirish.s@amd.com>
> ---
>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 -------------------
>   1 file changed, 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index af6adffba788..333f9904f135 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs amdgpu_dm_connector_funcs = {
>   	.atomic_get_property = amdgpu_dm_connector_atomic_get_property
>   };
>   
> -static struct drm_encoder *best_encoder(struct drm_connector *connector)
> -{
> -	int enc_id = connector->encoder_ids[0];
> -	struct drm_mode_object *obj;
> -	struct drm_encoder *encoder;
> -
> -	DRM_DEBUG_DRIVER("Finding the best encoder\n");
> -
> -	/* pick the encoder ids */
> -	if (enc_id) {
> -		obj = drm_mode_object_find(connector->dev, NULL, enc_id, DRM_MODE_OBJECT_ENCODER);
> -		if (!obj) {
> -			DRM_ERROR("Couldn't find a matching encoder for our connector\n");
> -			return NULL;
> -		}
> -		encoder = obj_to_encoder(obj);
> -		return encoder;
> -	}
> -	DRM_ERROR("No encoder id\n");
> -	return NULL;
> -}
> -
>   static int get_modes(struct drm_connector *connector)
>   {
>   	return amdgpu_dm_connector_get_modes(connector);
> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
>   	 */
>   	.get_modes = get_modes,
>   	.mode_valid = amdgpu_dm_connector_mode_valid,
> -	.best_encoder = best_encoder
>   };
>   
>   static void dm_crtc_helper_disable(struct drm_crtc *crtc)
>
Daniel Vetter Sept. 5, 2018, 1:48 p.m. UTC | #2
On Wed, Sep 5, 2018 at 3:45 PM, Leo Li <sunpeng.li@amd.com> wrote:
>
>
> On 2018-09-03 12:54 PM, Daniel Vetter wrote:
>>
>> For atomic driver this is the default, no need to reimplement it. We
>> still need to keep the copypasta for not-atomic drivers though, since
>> no one polished the legacy crtc helpers as much as the atomic ones.
>
>
> Thanks for the patch! It seems I made an identical change here:
> https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html
>
> One line difference though. I wasn't aware that the default is used when
> .best_encoder is null, so I just hooked it to the default helper.
>
> If it's OK with you, I'll do a follow-up to drop the hook, so we don't
> have two conflicting patches in flight.

I have a follow-up patches to unexport the helper, so would be good if
Alex sends out the pull request so I can sort this all out in
drm-misc-next. I don't want to split up the patch series if possible.
Or we just deal with the conflict, it's minor.

Alex?
-Daniel

> Leo
>
>
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: Harry Wentland <harry.wentland@amd.com>
>> Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> Cc: Tony Cheng <Tony.Cheng@amd.com>
>> Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>> Cc: Shirish S <shirish.s@amd.com>
>> ---
>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 -------------------
>>   1 file changed, 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index af6adffba788..333f9904f135 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs
>> amdgpu_dm_connector_funcs = {
>>         .atomic_get_property = amdgpu_dm_connector_atomic_get_property
>>   };
>>   -static struct drm_encoder *best_encoder(struct drm_connector
>> *connector)
>> -{
>> -       int enc_id = connector->encoder_ids[0];
>> -       struct drm_mode_object *obj;
>> -       struct drm_encoder *encoder;
>> -
>> -       DRM_DEBUG_DRIVER("Finding the best encoder\n");
>> -
>> -       /* pick the encoder ids */
>> -       if (enc_id) {
>> -               obj = drm_mode_object_find(connector->dev, NULL, enc_id,
>> DRM_MODE_OBJECT_ENCODER);
>> -               if (!obj) {
>> -                       DRM_ERROR("Couldn't find a matching encoder for
>> our connector\n");
>> -                       return NULL;
>> -               }
>> -               encoder = obj_to_encoder(obj);
>> -               return encoder;
>> -       }
>> -       DRM_ERROR("No encoder id\n");
>> -       return NULL;
>> -}
>> -
>>   static int get_modes(struct drm_connector *connector)
>>   {
>>         return amdgpu_dm_connector_get_modes(connector);
>> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
>>          */
>>         .get_modes = get_modes,
>>         .mode_valid = amdgpu_dm_connector_mode_valid,
>> -       .best_encoder = best_encoder
>>   };
>>     static void dm_crtc_helper_disable(struct drm_crtc *crtc)
>>
>
Deucher, Alexander Sept. 6, 2018, 4:33 p.m. UTC | #3
> -----Original Message-----
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> Sent: Wednesday, September 5, 2018 9:48 AM
> To: Li, Sun peng (Leo) <Sunpeng.Li@amd.com>
> Cc: DRI Development <dri-devel@lists.freedesktop.org>; Intel Graphics
> Development <intel-gfx@lists.freedesktop.org>; Daniel Vetter
> <daniel.vetter@intel.com>; Deucher, Alexander
> <Alexander.Deucher@amd.com>; Wentland, Harry
> <Harry.Wentland@amd.com>; Grodzovsky, Andrey
> <Andrey.Grodzovsky@amd.com>; Cheng, Tony <Tony.Cheng@amd.com>; S,
> Shirish <Shirish.S@amd.com>
> Subject: Re: [PATCH 08/14] drm/amdgpu: Remove default best_encoder
> hook from DC
> 
> On Wed, Sep 5, 2018 at 3:45 PM, Leo Li <sunpeng.li@amd.com> wrote:
> >
> >
> > On 2018-09-03 12:54 PM, Daniel Vetter wrote:
> >>
> >> For atomic driver this is the default, no need to reimplement it. We
> >> still need to keep the copypasta for not-atomic drivers though, since
> >> no one polished the legacy crtc helpers as much as the atomic ones.
> >
> >
> > Thanks for the patch! It seems I made an identical change here:
> > https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html
> >
> > One line difference though. I wasn't aware that the default is used
> > when .best_encoder is null, so I just hooked it to the default helper.
> >
> > If it's OK with you, I'll do a follow-up to drop the hook, so we don't
> > have two conflicting patches in flight.
> 
> I have a follow-up patches to unexport the helper, so would be good if Alex
> sends out the pull request so I can sort this all out in drm-misc-next. I don't
> want to split up the patch series if possible.
> Or we just deal with the conflict, it's minor.
> 
> Alex?

I'm planning to send the PR next week when I'm back in the office.  I can drop Leo's patch when I send the PR if that makes things easier.

Alex

> -Daniel
> 
> > Leo
> >
> >
> >>
> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> >> Cc: Alex Deucher <alexander.deucher@amd.com>
> >> Cc: Harry Wentland <harry.wentland@amd.com>
> >> Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> >> Cc: Tony Cheng <Tony.Cheng@amd.com>
> >> Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
> >> Cc: Shirish S <shirish.s@amd.com>
> >> ---
> >>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ----------------
> ---
> >>   1 file changed, 23 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> index af6adffba788..333f9904f135 100644
> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> >> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs
> >> amdgpu_dm_connector_funcs = {
> >>         .atomic_get_property =
> amdgpu_dm_connector_atomic_get_property
> >>   };
> >>   -static struct drm_encoder *best_encoder(struct drm_connector
> >> *connector)
> >> -{
> >> -       int enc_id = connector->encoder_ids[0];
> >> -       struct drm_mode_object *obj;
> >> -       struct drm_encoder *encoder;
> >> -
> >> -       DRM_DEBUG_DRIVER("Finding the best encoder\n");
> >> -
> >> -       /* pick the encoder ids */
> >> -       if (enc_id) {
> >> -               obj = drm_mode_object_find(connector->dev, NULL, enc_id,
> >> DRM_MODE_OBJECT_ENCODER);
> >> -               if (!obj) {
> >> -                       DRM_ERROR("Couldn't find a matching encoder for
> >> our connector\n");
> >> -                       return NULL;
> >> -               }
> >> -               encoder = obj_to_encoder(obj);
> >> -               return encoder;
> >> -       }
> >> -       DRM_ERROR("No encoder id\n");
> >> -       return NULL;
> >> -}
> >> -
> >>   static int get_modes(struct drm_connector *connector)
> >>   {
> >>         return amdgpu_dm_connector_get_modes(connector);
> >> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
> >>          */
> >>         .get_modes = get_modes,
> >>         .mode_valid = amdgpu_dm_connector_mode_valid,
> >> -       .best_encoder = best_encoder
> >>   };
> >>     static void dm_crtc_helper_disable(struct drm_crtc *crtc)
> >>
> >
> 
> 
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
Daniel Vetter Sept. 6, 2018, 10:12 p.m. UTC | #4
On Thu, Sep 6, 2018 at 6:33 PM, Deucher, Alexander
<Alexander.Deucher@amd.com> wrote:
>> -----Original Message-----
>> From: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Sent: Wednesday, September 5, 2018 9:48 AM
>> To: Li, Sun peng (Leo) <Sunpeng.Li@amd.com>
>> Cc: DRI Development <dri-devel@lists.freedesktop.org>; Intel Graphics
>> Development <intel-gfx@lists.freedesktop.org>; Daniel Vetter
>> <daniel.vetter@intel.com>; Deucher, Alexander
>> <Alexander.Deucher@amd.com>; Wentland, Harry
>> <Harry.Wentland@amd.com>; Grodzovsky, Andrey
>> <Andrey.Grodzovsky@amd.com>; Cheng, Tony <Tony.Cheng@amd.com>; S,
>> Shirish <Shirish.S@amd.com>
>> Subject: Re: [PATCH 08/14] drm/amdgpu: Remove default best_encoder
>> hook from DC
>>
>> On Wed, Sep 5, 2018 at 3:45 PM, Leo Li <sunpeng.li@amd.com> wrote:
>> >
>> >
>> > On 2018-09-03 12:54 PM, Daniel Vetter wrote:
>> >>
>> >> For atomic driver this is the default, no need to reimplement it. We
>> >> still need to keep the copypasta for not-atomic drivers though, since
>> >> no one polished the legacy crtc helpers as much as the atomic ones.
>> >
>> >
>> > Thanks for the patch! It seems I made an identical change here:
>> > https://lists.freedesktop.org/archives/amd-gfx/2018-August/026064.html
>> >
>> > One line difference though. I wasn't aware that the default is used
>> > when .best_encoder is null, so I just hooked it to the default helper.
>> >
>> > If it's OK with you, I'll do a follow-up to drop the hook, so we don't
>> > have two conflicting patches in flight.
>>
>> I have a follow-up patches to unexport the helper, so would be good if Alex
>> sends out the pull request so I can sort this all out in drm-misc-next. I don't
>> want to split up the patch series if possible.
>> Or we just deal with the conflict, it's minor.
>>
>> Alex?
>
> I'm planning to send the PR next week when I'm back in the office.  I can drop Leo's patch when I send the PR if that makes things easier.

I'm happy to rebase, just go ahead. I've added a bunch more patches to
my helper cleanup series anyway, so will take a bit longer to get all
landed.
-Daniel

>
> Alex
>
>> -Daniel
>>
>> > Leo
>> >
>> >
>> >>
>> >> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> >> Cc: Alex Deucher <alexander.deucher@amd.com>
>> >> Cc: Harry Wentland <harry.wentland@amd.com>
>> >> Cc: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>> >> Cc: Tony Cheng <Tony.Cheng@amd.com>
>> >> Cc: "Leo (Sunpeng) Li" <sunpeng.li@amd.com>
>> >> Cc: Shirish S <shirish.s@amd.com>
>> >> ---
>> >>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 ----------------
>> ---
>> >>   1 file changed, 23 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> index af6adffba788..333f9904f135 100644
>> >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> >> @@ -2794,28 +2794,6 @@ static const struct drm_connector_funcs
>> >> amdgpu_dm_connector_funcs = {
>> >>         .atomic_get_property =
>> amdgpu_dm_connector_atomic_get_property
>> >>   };
>> >>   -static struct drm_encoder *best_encoder(struct drm_connector
>> >> *connector)
>> >> -{
>> >> -       int enc_id = connector->encoder_ids[0];
>> >> -       struct drm_mode_object *obj;
>> >> -       struct drm_encoder *encoder;
>> >> -
>> >> -       DRM_DEBUG_DRIVER("Finding the best encoder\n");
>> >> -
>> >> -       /* pick the encoder ids */
>> >> -       if (enc_id) {
>> >> -               obj = drm_mode_object_find(connector->dev, NULL, enc_id,
>> >> DRM_MODE_OBJECT_ENCODER);
>> >> -               if (!obj) {
>> >> -                       DRM_ERROR("Couldn't find a matching encoder for
>> >> our connector\n");
>> >> -                       return NULL;
>> >> -               }
>> >> -               encoder = obj_to_encoder(obj);
>> >> -               return encoder;
>> >> -       }
>> >> -       DRM_ERROR("No encoder id\n");
>> >> -       return NULL;
>> >> -}
>> >> -
>> >>   static int get_modes(struct drm_connector *connector)
>> >>   {
>> >>         return amdgpu_dm_connector_get_modes(connector);
>> >> @@ -2934,7 +2912,6 @@ amdgpu_dm_connector_helper_funcs = {
>> >>          */
>> >>         .get_modes = get_modes,
>> >>         .mode_valid = amdgpu_dm_connector_mode_valid,
>> >> -       .best_encoder = best_encoder
>> >>   };
>> >>     static void dm_crtc_helper_disable(struct drm_crtc *crtc)
>> >>
>> >
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index af6adffba788..333f9904f135 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2794,28 +2794,6 @@  static const struct drm_connector_funcs amdgpu_dm_connector_funcs = {
 	.atomic_get_property = amdgpu_dm_connector_atomic_get_property
 };
 
-static struct drm_encoder *best_encoder(struct drm_connector *connector)
-{
-	int enc_id = connector->encoder_ids[0];
-	struct drm_mode_object *obj;
-	struct drm_encoder *encoder;
-
-	DRM_DEBUG_DRIVER("Finding the best encoder\n");
-
-	/* pick the encoder ids */
-	if (enc_id) {
-		obj = drm_mode_object_find(connector->dev, NULL, enc_id, DRM_MODE_OBJECT_ENCODER);
-		if (!obj) {
-			DRM_ERROR("Couldn't find a matching encoder for our connector\n");
-			return NULL;
-		}
-		encoder = obj_to_encoder(obj);
-		return encoder;
-	}
-	DRM_ERROR("No encoder id\n");
-	return NULL;
-}
-
 static int get_modes(struct drm_connector *connector)
 {
 	return amdgpu_dm_connector_get_modes(connector);
@@ -2934,7 +2912,6 @@  amdgpu_dm_connector_helper_funcs = {
 	 */
 	.get_modes = get_modes,
 	.mode_valid = amdgpu_dm_connector_mode_valid,
-	.best_encoder = best_encoder
 };
 
 static void dm_crtc_helper_disable(struct drm_crtc *crtc)