Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c --- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c 2008-09-10 17:40:52.000000000 +0800 +++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c 2008-09-10 17:49:31.000000000 +0800 @@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;
Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch description again. This helps my patch work a lot indeed.
thanks,
Takashi
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer when testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c 2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;
Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch description again. This helps my patch work a lot indeed.
thanks,
Takashi ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
Hi, Takashi
About the "long delay", it seemed that we need to add this "delay" to fix some issue on our aza controller. Because it related with the controller, not just codec, so I think this flag should not be added in codec-initialization part. I think it's better to add the flag in that place which I submitted.
Thanks
-----Original Message----- From: Wei Ni Sent: Friday, September 19, 2008 12:43 PM To: 'Takashi Iwai'; peerchen Cc: Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: RE: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer when testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c 2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;
Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch description again. This helps my patch work a lot indeed.
thanks,
Takashi ----------------------------------------------------------------------------------- This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message. -----------------------------------------------------------------------------------
At Mon, 22 Sep 2008 14:33:58 +0800, Wei Ni wrote:
Hi, Takashi
About the "long delay", it seemed that we need to add this "delay" to fix some issue on our aza controller. Because it related with the controller, not just codec, so I think this flag should not be added in codec-initialization part. I think it's better to add the flag in that place which I submitted.
OK, then could you split the patch to separate ones, i.e. one for the fix for this problem regarding Nvidia controller chip, and one for the addition of HDMI?
Don't forget to add a proper description for each patch, please ;)
As mentioned, that flag is a last resort, and I think it can be fixed in a saner way. But, the flag is surely safe, and we can track down the problem later on.
thanks,
Takashi
Thanks
-----Original Message----- From: Wei Ni Sent: Friday, September 19, 2008 12:43 PM To: 'Takashi Iwai'; peerchen Cc: Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: RE: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer when testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c 2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch description again. This helps my patch work a lot indeed.
thanks,
Takashi
This email message is for the sole use of the intended recipient(s) and may contain confidential information. Any unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply email and destroy all copies of the original message.
Hi, Takashi I use git to generate patches. (I run git-fetch origin, git-fetch-rebase origin, and git-format-patch origin)
There are two patch, one for the addition of HDMI, and one for the fix for our aza controller.
Please check it.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, September 22, 2008 5:58 PM To: Wei Ni Cc: peerchen; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Mon, 22 Sep 2008 14:33:58 +0800, Wei Ni wrote:
Hi, Takashi
About the "long delay", it seemed that we need to add this "delay" to fix some issue on our aza controller. Because it related with the controller, not just codec, so I think
this
flag should not be added in codec-initialization part. I think it's better to add the flag in that place which I submitted.
OK, then could you split the patch to separate ones, i.e. one for the fix for this problem regarding Nvidia controller chip, and one for the addition of HDMI?
Don't forget to add a proper description for each patch, please ;)
As mentioned, that flag is a last resort, and I think it can be fixed in a saner way. But, the flag is surely safe, and we can track down the problem later on.
thanks,
Takashi
Thanks
-----Original Message----- From: Wei Ni Sent: Friday, September 19, 2008 12:43 PM To: 'Takashi Iwai'; peerchen Cc: Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: RE: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer
when
testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c 2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch description
again. This helps my patch work a lot indeed.
thanks,
Takashi
------------------------------------------------------------------------ -----------
This email message is for the sole use of the intended recipient(s)
and may contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact
the sender by
reply email and destroy all copies of the original message.
------------------------------------------------------------------------ -----------
At Thu, 25 Sep 2008 15:14:50 +0800, Wei Ni wrote:
Hi, Takashi I use git to generate patches. (I run git-fetch origin, git-fetch-rebase origin, and git-format-patch origin)
There are two patch, one for the addition of HDMI, and one for the fix for our aza controller.
Please check it.
Thanks for patches. It's good that you are using git now. The code changes look good, but there are small issues in the patches:
- The author line is bogus. Edit your ~/.gitconfig and add like the following: [user] name = Wei Ni email = wni@nvidia.com Then reset and commit patches again.
- Missing sign-off. Add --sign option when you do git-commit.
- No proper change log. Only a subject line isn't enough for many (most) cases, especially for a patch like workaround for nvidia controller. Please describe why this change is needed more precisely.
Could you fix and repost again?
Thanks!
Takashi
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, September 22, 2008 5:58 PM To: Wei Ni Cc: peerchen; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Mon, 22 Sep 2008 14:33:58 +0800, Wei Ni wrote:
Hi, Takashi
About the "long delay", it seemed that we need to add this "delay" to fix some issue on our aza controller. Because it related with the controller, not just codec, so I think
this
flag should not be added in codec-initialization part. I think it's better to add the flag in that place which I submitted.
OK, then could you split the patch to separate ones, i.e. one for the fix for this problem regarding Nvidia controller chip, and one for the addition of HDMI?
Don't forget to add a proper description for each patch, please ;)
As mentioned, that flag is a last resort, and I think it can be fixed in a saner way. But, the flag is surely safe, and we can track down the problem later on.
thanks,
Takashi
Thanks
-----Original Message----- From: Wei Ni Sent: Friday, September 19, 2008 12:43 PM To: 'Takashi Iwai'; peerchen Cc: Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: RE: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer
when
testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c 2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch description
again. This helps my patch work a lot indeed.
thanks,
Takashi
This email message is for the sole use of the intended recipient(s)
and may contain
confidential information. Any unauthorized review, use, disclosure or
distribution
is prohibited. If you are not the intended recipient, please contact
the sender by
reply email and destroy all copies of the original message.
[2 0001-Support-NVIDIA-MCP78-HDMI-audio.patch <application/octet-stream (base64)>]
[3 0002-Fix-for-reading-RIRB-buffer-on-NVIDIA-aza-controller.patch <application/octet-stream (base64)>]
Thanks for your comments, I will fix and repost again.
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, September 25, 2008 5:55 PM To: Wei Ni Cc: peerchen; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Thu, 25 Sep 2008 15:14:50 +0800, Wei Ni wrote:
Hi, Takashi I use git to generate patches. (I run git-fetch origin,
git-fetch-rebase
origin, and git-format-patch origin)
There are two patch, one for the addition of HDMI, and one for the fix for our aza controller.
Please check it.
Thanks for patches. It's good that you are using git now. The code changes look good, but there are small issues in the patches:
- The author line is bogus. Edit your ~/.gitconfig and add like the following: [user] name = Wei Ni email = wni@nvidia.com Then reset and commit patches again.
- Missing sign-off. Add --sign option when you do git-commit.
- No proper change log. Only a subject line isn't enough for many (most) cases, especially for a patch like workaround for nvidia controller. Please describe why this change is needed more precisely.
Could you fix and repost again?
Thanks!
Takashi
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, September 22, 2008 5:58 PM To: Wei Ni Cc: peerchen; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Mon, 22 Sep 2008 14:33:58 +0800, Wei Ni wrote:
Hi, Takashi
About the "long delay", it seemed that we need to add this "delay"
to
fix some issue on our aza controller. Because it related with the controller, not just codec, so I think
this
flag should not be added in codec-initialization part. I think it's better to add the flag in that place which I submitted.
OK, then could you split the patch to separate ones, i.e. one for the fix for this problem regarding Nvidia controller chip, and one for the addition of HDMI?
Don't forget to add a proper description for each patch, please ;)
As mentioned, that flag is a last resort, and I think it can be fixed in a saner way. But, the flag is surely safe, and we can track down the problem later on.
thanks,
Takashi
Thanks
-----Original Message----- From: Wei Ni Sent: Friday, September 19, 2008 12:43 PM To: 'Takashi Iwai'; peerchen Cc: Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: RE: [alsa-devel] [PATCH] add the nvidia HDMI codec
driverfor
MCP77/79
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer
when
testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec
driverfor
MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c
2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch
description
again. This helps my patch work a lot indeed.
thanks,
Takashi
------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s)
and may contain
confidential information. Any unauthorized review, use, disclosure
or
distribution
is prohibited. If you are not the intended recipient, please
contact
the sender by
reply email and destroy all copies of the original message.
------------------------------------------------------------------------
[2 0001-Support-NVIDIA-MCP78-HDMI-audio.patch
<application/octet-stream (base64)>]
[3 0002-Fix-for-reading-RIRB-buffer-on-NVIDIA-aza-controller.patch
<application/octet-stream (base64)>]
Hi,
did we forgot alsa-driver/pci/hda/patch_nvhdmi.c file?
this is my first posting to the list so be tolerant, please.
regards,
At Fri, 26 Sep 2008 12:22:51 +0200, Pawel Krzesniak wrote:
Hi,
did we forgot alsa-driver/pci/hda/patch_nvhdmi.c file?
this is my first posting to the list so be tolerant, please.
Heh, you're right. Forgot that one. Now pushed out to my git tree.
thanks,
Takashi
Hi, Takashi I fixed something based on your comments, please check the patch files.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Thursday, September 25, 2008 5:55 PM To: Wei Ni Cc: peerchen; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Thu, 25 Sep 2008 15:14:50 +0800, Wei Ni wrote:
Hi, Takashi I use git to generate patches. (I run git-fetch origin,
git-fetch-rebase
origin, and git-format-patch origin)
There are two patch, one for the addition of HDMI, and one for the fix for our aza controller.
Please check it.
Thanks for patches. It's good that you are using git now. The code changes look good, but there are small issues in the patches:
- The author line is bogus. Edit your ~/.gitconfig and add like the following: [user] name = Wei Ni email = wni@nvidia.com Then reset and commit patches again.
- Missing sign-off. Add --sign option when you do git-commit.
- No proper change log. Only a subject line isn't enough for many (most) cases, especially for a patch like workaround for nvidia controller. Please describe why this change is needed more precisely.
Could you fix and repost again?
Thanks!
Takashi
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Monday, September 22, 2008 5:58 PM To: Wei Ni Cc: peerchen; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec driverfor MCP77/79
At Mon, 22 Sep 2008 14:33:58 +0800, Wei Ni wrote:
Hi, Takashi
About the "long delay", it seemed that we need to add this "delay"
to
fix some issue on our aza controller. Because it related with the controller, not just codec, so I think
this
flag should not be added in codec-initialization part. I think it's better to add the flag in that place which I submitted.
OK, then could you split the patch to separate ones, i.e. one for the fix for this problem regarding Nvidia controller chip, and one for the addition of HDMI?
Don't forget to add a proper description for each patch, please ;)
As mentioned, that flag is a last resort, and I think it can be fixed in a saner way. But, the flag is surely safe, and we can track down the problem later on.
thanks,
Takashi
Thanks
-----Original Message----- From: Wei Ni Sent: Friday, September 19, 2008 12:43 PM To: 'Takashi Iwai'; peerchen Cc: Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: RE: [alsa-devel] [PATCH] add the nvidia HDMI codec
driverfor
MCP77/79
Thanks for your reply.
About the "long delay", we had some problems on reading RIRB buffer
when
testing this driver, so we add this "delay" We will try to find a best way to fix this issue.
And we didn't familiar with git, we will research it more such as "rebase" and then submit the patch file again.
Thanks
-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Wednesday, September 17, 2008 10:05 PM To: peerchen Cc: Wei Ni; Pavel Hofman; Peer Chen; alsa-devel; linux-kernel; akpm Subject: Re: [alsa-devel] [PATCH] add the nvidia HDMI codec
driverfor
MCP77/79
At Wed, 17 Sep 2008 17:00:04 +0800, peerchen wrote:
new patch base on git tree.
Signed-off-by: Wei Ni wni@nvidia.com Signed-off-by: Peer Chen peerchen@gmail.com
Thanks!
But, the patch looks like no "rebase". You secretly added a new function call that wasn't in your previous patch, namely...
diff -uprN -X linux-2.6-tiwai-sound/Documentation/dontdiff
linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
--- linux-2.6-tiwai-sound/sound/pci/hda/hda_intel.c
2008-09-10
17:40:52.000000000 +0800
+++ linux-2.6-tiwai-sound-niwei/sound/pci/hda/hda_intel.c
2008-09-10 17:49:31.000000000 +0800
@@ -1220,6 +1220,9 @@ static int __devinit azx_codec_create(st if (err < 0) return err;
- if (chip->driver_type == AZX_DRIVER_NVIDIA)
chip->bus->needs_damn_long_delay = 1;Do we really need this inevitably? I don't think so -- otherwise I would have far more bug reports.
This flag is the last resort and should be avoided as much as possible. This results in a significant slow down of suspend/resume speed, for example.
If it's really needed with some devices and there is no other way to fix it, add this flag rather in the codec-initialization part.
Also, if you resend a patch, please add the original patch
description
again. This helps my patch work a lot indeed.
thanks,
Takashi
------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s)
and may contain
confidential information. Any unauthorized review, use, disclosure
or
distribution
is prohibited. If you are not the intended recipient, please
contact
the sender by
reply email and destroy all copies of the original message.
------------------------------------------------------------------------
[2 0001-Support-NVIDIA-MCP78-HDMI-audio.patch
<application/octet-stream (base64)>]
[3 0002-Fix-for-reading-RIRB-buffer-on-NVIDIA-aza-controller.patch
<application/octet-stream (base64)>]
At Fri, 26 Sep 2008 14:19:46 +0800, Wei Ni wrote:
Hi, Takashi I fixed something based on your comments, please check the patch files.
Thanks! I applied both patches now after formatting comments. These will be included in 2.6.28.
(At the next time, please put your sign-off at the end of commit log, and add line breaks appropriately to keep commit log in 80 or less column letters.)
Takashi
participants (3)
-
Pawel Krzesniak -
Takashi Iwai -
Wei Ni